Le mer. 19 déc. 2018 00:16, Michael S. Tsirkin <m...@redhat.com> a écrit :
> On Tue, Dec 18, 2018 at 06:51:22PM +0100, Philippe Mathieu-Daudé wrote: > > GCC 8 introduced the -Wstringop-overflow, which detect buffer overflow > > by string-modifying functions declared in <string.h>, such strncpy(), > > used in global_state_store_running(). > > > > Since the global_state.runstate does not necessarily contains a > > terminating NUL character, We had to use the QEMU_NONSTRING attribute. > > > > The GCC manual says about the nonstring attribute: > > > > However, when the array is declared with the attribute the call to > > strlen is diagnosed because when the array doesn’t contain a > > NUL-terminated string the call is undefined. [...] > > In addition, calling strnlen and strndup with such arrays is safe > > provided a suitable bound is specified, and not diagnosed. > > > > GCC indeed found an incorrect use of strlen(), because this array > > is loaded by VMSTATE_BUFFER(runstate, GlobalState) then parsed > > using qapi_enum_parse which does not get the buffer length. > > > > Use strnlen() which returns sizeof(s->runstate) if the array is not > > NUL-terminated. > > > > This fixes: > > > > CC migration/global_state.o > > qemu/migration/global_state.c: In function 'global_state_pre_save': > > qemu/migration/global_state.c:109:15: error: 'strlen' argument 1 > declared attribute 'nonstring' [-Werror=stringop-overflow=] > > s->size = strlen((char *)s->runstate) + 1; > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~ > > qemu/migration/global_state.c:24:13: note: argument 'runstate' > declared here > > uint8_t runstate[100] QEMU_NONSTRING; > > ^~~~~~~~ > > cc1: all warnings being treated as errors > > make: *** [qemu/rules.mak:69: migration/global_state.o] Error 1 > > > > Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com> > > --- > > migration/global_state.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/migration/global_state.c b/migration/global_state.c > > index 6e19333422..c19030ef62 100644 > > --- a/migration/global_state.c > > +++ b/migration/global_state.c > > @@ -106,7 +106,7 @@ static int global_state_pre_save(void *opaque) > > GlobalState *s = opaque; > > > > trace_migrate_global_state_pre_save((char *)s->runstate); > > - s->size = strlen((char *)s->runstate) + 1; > > + s->size = strnlen((char *)s->runstate, sizeof(s->runstate)) + 1; > > > > return 0; > > I don't think this works correctly if strnlen returns > sizeof(s->runstate). Which never happens so we probably should > jus add > > assert(e->size is <= sizeof(s->runstate)); > OK I'll resend Marc-André previous patch. > But also I think this is not enough, there's a problem in post-load > in the call to qapi_enum_parse. You probably want to force > the last character to 0 there. > OK, I'll have a look there. > > > } > > -- > > 2.17.2 >