On 18/12/18 18:17, Michael S. Tsirkin wrote: > On Tue, Dec 18, 2018 at 06:12:05PM +0100, Paolo Bonzini wrote: >> On 18/12/18 17:55, Philippe Mathieu-Daudé wrote: >>>> strpadcpy will instead just silence the warning. >>> migration/global_state.c:109:15: error: 'strlen' argument 1 declared >>> attribute 'nonstring' [-Werror=stringop-overflow=] >>> s->size = strlen((char *)s->runstate) + 1; >>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> GCC won... It is true this strlen() is buggy, indeed s->runstate might >>> be not NUL-terminated. >> >> No, runstate is declared as an array of 100 bytes, which are more than >> enough. It's ugly code but not buggy. >> >> Paolo > > Yes ... but it is loaded using > VMSTATE_BUFFER(runstate, GlobalState), > and parsed using qapi_enum_parse which does not get > the buffer length. > > So unless we are lucky there's a buffer overrun > on a remote/file input here. > > Seems buggy to me - what am I missing?
Yup. I think we're lucky twice though. First, the state field stops the runaway qapi_enum_parse. Second, in any case worst case it's a segv on migration. This is a bug but not a CVE. Paolo
