| Message ID | 1260385471-8561-1-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
Anthony Liguori <aliguori@us.ibm.com> writes: > qemu_malloc() does not allow size=0 to be passed in and aborts on this behavior. > > Unfortunately, there is good reason to believe that within qemu, there are a > number of, so far, undetected places that assume size=0 can be safely passed. > Since we do not want to abort unnecessarily in production builds, return > qemu_malloc(1) whenever the version file indicates that this is a production > build. > > Also introduce --enable-zero-malloc/--disable-zero-malloc to make this behavior > overridable. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > configure | 24 +++++++++++++++++++++++- > qemu-malloc.c | 17 +++++++++++++---- > 2 files changed, 36 insertions(+), 5 deletions(-) > [...] > diff --git a/qemu-malloc.c b/qemu-malloc.c > index 295d185..e82af26 100644 > --- a/qemu-malloc.c > +++ b/qemu-malloc.c > @@ -42,21 +42,30 @@ void qemu_free(void *ptr) > free(ptr); > } > > +static int allow_zero_malloc(void) > +{ > +#if defined(CONFIG_ZERO_MALLOC) > + return 1; > +#else > + return 0; > +#endif > +} > + > void *qemu_malloc(size_t size) > { > - if (!size) { > + if (!size && !allow_zero_malloc()) { > abort(); > } > - return oom_check(malloc(size)); > + return oom_check(malloc(size ? size : 1)); > } > > void *qemu_realloc(void *ptr, size_t size) > { > if (size) { > return oom_check(realloc(ptr, size)); > - } else { > + } else if (allow_zero_malloc()) { > if (ptr) { > - return realloc(ptr, size); > + return realloc(ptr, size ? size : 1); > } > } > abort(); This still aborts on qemu_realloc(NULL, 0), even with CONFIG_ZERO_MALLOC. Intentional?
Markus Armbruster wrote: > Anthony Liguori <aliguori@us.ibm.com> writes: > > >> qemu_malloc() does not allow size=0 to be passed in and aborts on this behavior. >> >> Unfortunately, there is good reason to believe that within qemu, there are a >> number of, so far, undetected places that assume size=0 can be safely passed. >> Since we do not want to abort unnecessarily in production builds, return >> qemu_malloc(1) whenever the version file indicates that this is a production >> build. >> >> Also introduce --enable-zero-malloc/--disable-zero-malloc to make this behavior >> overridable. >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> --- >> configure | 24 +++++++++++++++++++++++- >> qemu-malloc.c | 17 +++++++++++++---- >> 2 files changed, 36 insertions(+), 5 deletions(-) >> >> > [...] > >> diff --git a/qemu-malloc.c b/qemu-malloc.c >> index 295d185..e82af26 100644 >> --- a/qemu-malloc.c >> +++ b/qemu-malloc.c >> @@ -42,21 +42,30 @@ void qemu_free(void *ptr) >> free(ptr); >> } >> >> +static int allow_zero_malloc(void) >> +{ >> +#if defined(CONFIG_ZERO_MALLOC) >> + return 1; >> +#else >> + return 0; >> +#endif >> +} >> + >> void *qemu_malloc(size_t size) >> { >> - if (!size) { >> + if (!size && !allow_zero_malloc()) { >> abort(); >> } >> - return oom_check(malloc(size)); >> + return oom_check(malloc(size ? size : 1)); >> } >> >> void *qemu_realloc(void *ptr, size_t size) >> { >> if (size) { >> return oom_check(realloc(ptr, size)); >> - } else { >> + } else if (allow_zero_malloc()) { >> if (ptr) { >> - return realloc(ptr, size); >> + return realloc(ptr, size ? size : 1); >> } >> } >> abort(); >> > > This still aborts on qemu_realloc(NULL, 0), even with > CONFIG_ZERO_MALLOC. Intentional? > I guess not. Should it? Seems like a very strange case.. Regards, Anthony Liguori
Anthony Liguori <aliguori@linux.vnet.ibm.com> writes: > Markus Armbruster wrote: >> This still aborts on qemu_realloc(NULL, 0), even with >> CONFIG_ZERO_MALLOC. Intentional? >> > I guess not. Should it? Seems like a very strange case.. It is a strange case, but I think the point of this commit is not to abort on conditions perceived strange ;) I think it should follow C89 and behave exactly like qemu_malloc(0).
diff --git a/configure b/configure index a29839e..ede820e 100755 --- a/configure +++ b/configure @@ -256,6 +256,7 @@ blobs="yes" pkgversion="" check_utests="no" user_pie="no" +zero_malloc="" # OS specific if check_define __linux__ ; then @@ -600,6 +601,10 @@ for opt do ;; --enable-docs) docs="yes" ;; + --enable-zero-malloc) zero_malloc="yes" + ;; + --disable-zero-malloc) zero_malloc="no" + ;; *) echo "ERROR: unknown option $opt"; show_help="yes" ;; esac @@ -752,6 +757,8 @@ echo " --enable-linux-aio enable Linux AIO support" echo " --enable-io-thread enable IO thread" echo " --disable-blobs disable installing provided firmware blobs" echo " --kerneldir=PATH look for kernel includes in PATH" +echo " --enable-zero-malloc allow allocations of zero size" +echo " --disable-zero-malloc abort on allocations of zero size" echo "" echo "NOTE: The object files are built at the place where configure is launched" exit 1 @@ -1792,8 +1799,9 @@ fi # Consult white-list to determine whether to enable werror # by default. Only enable by default for git builds +z_version=`cut -f3 -d. $source_path/VERSION` + if test -z "$werror" ; then - z_version=`cut -f3 -d. $source_path/VERSION` if test "$z_version" = "50" -a \ "$linux" = "yes" ; then werror="yes" @@ -1802,6 +1810,16 @@ if test -z "$werror" ; then fi fi +# Disable zero malloc errors for official releases unless explicitly told to +# enable/disable +if test -z "$zero_malloc" ; then + if test "$z_version" = "50" ; then + zero_malloc="no" + else + zero_malloc="yes" + fi +fi + if test "$werror" = "yes" ; then QEMU_CFLAGS="-Werror $QEMU_CFLAGS" fi @@ -2109,6 +2127,10 @@ fi echo "CONFIG_UNAME_RELEASE=\"$uname_release\"" >> $config_host_mak +if test "$zero_malloc" = "yes" ; then + echo "CONFIG_ZERO_MALLOC=y" >> $config_host_mak +fi + # USB host support case "$usb" in linux) diff --git a/qemu-malloc.c b/qemu-malloc.c index 295d185..e82af26 100644 --- a/qemu-malloc.c +++ b/qemu-malloc.c @@ -42,21 +42,30 @@ void qemu_free(void *ptr) free(ptr); } +static int allow_zero_malloc(void) +{ +#if defined(CONFIG_ZERO_MALLOC) + return 1; +#else + return 0; +#endif +} + void *qemu_malloc(size_t size) { - if (!size) { + if (!size && !allow_zero_malloc()) { abort(); } - return oom_check(malloc(size)); + return oom_check(malloc(size ? size : 1)); } void *qemu_realloc(void *ptr, size_t size) { if (size) { return oom_check(realloc(ptr, size)); - } else { + } else if (allow_zero_malloc()) { if (ptr) { - return realloc(ptr, size); + return realloc(ptr, size ? size : 1); } } abort();
qemu_malloc() does not allow size=0 to be passed in and aborts on this behavior. Unfortunately, there is good reason to believe that within qemu, there are a number of, so far, undetected places that assume size=0 can be safely passed. Since we do not want to abort unnecessarily in production builds, return qemu_malloc(1) whenever the version file indicates that this is a production build. Also introduce --enable-zero-malloc/--disable-zero-malloc to make this behavior overridable. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- configure | 24 +++++++++++++++++++++++- qemu-malloc.c | 17 +++++++++++++---- 2 files changed, 36 insertions(+), 5 deletions(-)