| Message ID | 20200921093325.25617-1-ani@anisinha.ca (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] qom: code hardening - have bound checking while looping with integer value | expand |
Request to queue this patch for the next pull. On Mon, Sep 21, 2020 at 15:03 Ani Sinha <ani@anisinha.ca> wrote: > Object property insertion code iterates over an integer to get an unused > index that can be used as an unique name for an object property. This loop > increments the integer value indefinitely. Although very unlikely, this can > still cause an integer overflow. > In this change, we fix the above code by checking against INT16_MAX and > making > sure that the interger index does not overflow beyond that value. If no > available index is found, the code would cause an assertion failure. This > assertion failure is necessary because the callers of the function do not > check > the return value for NULL. > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > --- > qom/object.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > changelog: > v1: initial version > v2: change INT_MAX to INT16_MAX in code > v3: make the same change in commit log. Sorry for missing it. > > diff --git a/qom/object.c b/qom/object.c > index 387efb25eb..9962874598 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1166,11 +1166,11 @@ object_property_try_add(Object *obj, const char > *name, const char *type, > > if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) { > int i; > - ObjectProperty *ret; > + ObjectProperty *ret = NULL; > char *name_no_array = g_strdup(name); > > name_no_array[name_len - 3] = '\0'; > - for (i = 0; ; ++i) { > + for (i = 0; i < INT16_MAX; ++i) { > char *full_name = g_strdup_printf("%s[%d]", name_no_array, i); > > ret = object_property_try_add(obj, full_name, type, get, set, > @@ -1181,6 +1181,7 @@ object_property_try_add(Object *obj, const char > *name, const char *type, > } > } > g_free(name_no_array); > + assert(ret); > return ret; > } > > -- > 2.17.1 > >
Ping ... On Mon, Oct 12, 2020 at 8:08 PM Ani Sinha <ani@anisinha.ca> wrote: > > Request to queue this patch for the next pull. > > On Mon, Sep 21, 2020 at 15:03 Ani Sinha <ani@anisinha.ca> wrote: >> >> Object property insertion code iterates over an integer to get an unused >> index that can be used as an unique name for an object property. This loop >> increments the integer value indefinitely. Although very unlikely, this can >> still cause an integer overflow. >> In this change, we fix the above code by checking against INT16_MAX and making >> sure that the interger index does not overflow beyond that value. If no >> available index is found, the code would cause an assertion failure. This >> assertion failure is necessary because the callers of the function do not check >> the return value for NULL. >> >> Signed-off-by: Ani Sinha <ani@anisinha.ca> >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >> --- >> qom/object.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> changelog: >> v1: initial version >> v2: change INT_MAX to INT16_MAX in code >> v3: make the same change in commit log. Sorry for missing it. >> >> diff --git a/qom/object.c b/qom/object.c >> index 387efb25eb..9962874598 100644 >> --- a/qom/object.c >> +++ b/qom/object.c >> @@ -1166,11 +1166,11 @@ object_property_try_add(Object *obj, const char *name, const char *type, >> >> if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) { >> int i; >> - ObjectProperty *ret; >> + ObjectProperty *ret = NULL; >> char *name_no_array = g_strdup(name); >> >> name_no_array[name_len - 3] = '\0'; >> - for (i = 0; ; ++i) { >> + for (i = 0; i < INT16_MAX; ++i) { >> char *full_name = g_strdup_printf("%s[%d]", name_no_array, i); >> >> ret = object_property_try_add(obj, full_name, type, get, set, >> @@ -1181,6 +1181,7 @@ object_property_try_add(Object *obj, const char *name, const char *type, >> } >> } >> g_free(name_no_array); >> + assert(ret); >> return ret; >> } >> >> -- >> 2.17.1 >>
On Mon, Sep 21, 2020 at 03:03:25PM +0530, Ani Sinha wrote: > Object property insertion code iterates over an integer to get an unused > index that can be used as an unique name for an object property. This loop > increments the integer value indefinitely. Although very unlikely, this can > still cause an integer overflow. > In this change, we fix the above code by checking against INT16_MAX and making > sure that the interger index does not overflow beyond that value. If no > available index is found, the code would cause an assertion failure. This > assertion failure is necessary because the callers of the function do not check > the return value for NULL. > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Queued on machine-next, thanks! My apologies for the delay.
On Thu, Oct 15, 2020 at 10:22 PM Eduardo Habkost <ehabkost@redhat.com> wrote: > > On Mon, Sep 21, 2020 at 03:03:25PM +0530, Ani Sinha wrote: > > Object property insertion code iterates over an integer to get an unused > > index that can be used as an unique name for an object property. This loop > > increments the integer value indefinitely. Although very unlikely, this can > > still cause an integer overflow. > > In this change, we fix the above code by checking against INT16_MAX and making > > sure that the interger index does not overflow beyond that value. If no > > available index is found, the code would cause an assertion failure. This > > assertion failure is necessary because the callers of the function do not check > > the return value for NULL. > > > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > Queued on machine-next, thanks! My apologies for the delay. Any idea when this will be pulled?
Ping ... On Sat, Oct 31, 2020 at 9:51 PM Ani Sinha <ani@anisinha.ca> wrote: > On Thu, Oct 15, 2020 at 10:22 PM Eduardo Habkost <ehabkost@redhat.com> > wrote: > > > > On Mon, Sep 21, 2020 at 03:03:25PM +0530, Ani Sinha wrote: > > > Object property insertion code iterates over an integer to get an > unused > > > index that can be used as an unique name for an object property. This > loop > > > increments the integer value indefinitely. Although very unlikely, > this can > > > still cause an integer overflow. > > > In this change, we fix the above code by checking against INT16_MAX > and making > > > sure that the interger index does not overflow beyond that value. If no > > > available index is found, the code would cause an assertion failure. > This > > > assertion failure is necessary because the callers of the function do > not check > > > the return value for NULL. > > > > > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > > Queued on machine-next, thanks! My apologies for the delay. > > Any idea when this will be pulled? >
On Sat, Oct 31, 2020 at 09:51:38PM +0530, Ani Sinha wrote: > On Thu, Oct 15, 2020 at 10:22 PM Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > On Mon, Sep 21, 2020 at 03:03:25PM +0530, Ani Sinha wrote: > > > Object property insertion code iterates over an integer to get an unused > > > index that can be used as an unique name for an object property. This loop > > > increments the integer value indefinitely. Although very unlikely, this can > > > still cause an integer overflow. > > > In this change, we fix the above code by checking against INT16_MAX and making > > > sure that the interger index does not overflow beyond that value. If no > > > available index is found, the code would cause an assertion failure. This > > > assertion failure is necessary because the callers of the function do not check > > > the return value for NULL. > > > > > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > > Queued on machine-next, thanks! My apologies for the delay. > > Any idea when this will be pulled? I was planning to send a pull request before soft freeze, (October 27) but this was the only patch in the queue at that moment, so I decided to wait. As we're beyond freeze now, the pull request will be sent immediately after 5.2.0 is released.
On Wed, Nov 4, 2020 at 21:29 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Sat, Oct 31, 2020 at 09:51:38PM +0530, Ani Sinha wrote: > > On Thu, Oct 15, 2020 at 10:22 PM Eduardo Habkost <ehabkost@redhat.com> > wrote: > > > > > > On Mon, Sep 21, 2020 at 03:03:25PM +0530, Ani Sinha wrote: > > > > Object property insertion code iterates over an integer to get an > unused > > > > index that can be used as an unique name for an object property. > This loop > > > > increments the integer value indefinitely. Although very unlikely, > this can > > > > still cause an integer overflow. > > > > In this change, we fix the above code by checking against INT16_MAX > and making > > > > sure that the interger index does not overflow beyond that value. If > no > > > > available index is found, the code would cause an assertion failure. > This > > > > assertion failure is necessary because the callers of the function > do not check > > > > the return value for NULL. > > > > > > > > Signed-off-by: Ani Sinha <ani@anisinha.ca> > > > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > > > > > Queued on machine-next, thanks! My apologies for the delay. > > > > Any idea when this will be pulled? > > I was planning to send a pull request before soft freeze, > (October 27) but this was the only patch in the queue at that > moment, so I decided to wait. > > As we're beyond freeze now, the pull request will be sent > immediately after 5.2.0 is released. Gentle reminder since 5.2.0 has now been released. > >
diff --git a/qom/object.c b/qom/object.c index 387efb25eb..9962874598 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1166,11 +1166,11 @@ object_property_try_add(Object *obj, const char *name, const char *type, if (name_len >= 3 && !memcmp(name + name_len - 3, "[*]", 4)) { int i; - ObjectProperty *ret; + ObjectProperty *ret = NULL; char *name_no_array = g_strdup(name); name_no_array[name_len - 3] = '\0'; - for (i = 0; ; ++i) { + for (i = 0; i < INT16_MAX; ++i) { char *full_name = g_strdup_printf("%s[%d]", name_no_array, i); ret = object_property_try_add(obj, full_name, type, get, set, @@ -1181,6 +1181,7 @@ object_property_try_add(Object *obj, const char *name, const char *type, } } g_free(name_no_array); + assert(ret); return ret; }