On 7/19/19 10:03 AM, Eric Blake wrote: > We've had two separate reports of a caller running into use of > uninitialized data if s->quit is set (one detected by gcc -O3, another > by valgrind), due to checking 'nbd_reply_is_simple(reply) || s->quit' > in the wrong order. Rather than chasing down which callers need to > pre-initialize reply, it's easier to guarantee that reply will always > be set by nbd_co_receive_one_chunk() even on failure. > > Reported-by: Thomas Huth <[email protected]> > Reported-by: Andrey Shinkevich <[email protected]> > Signed-off-by: Eric Blake <[email protected]> > --- >
Blech. Needs a v2. Expanding context:
> +++ b/block/nbd.c
> @@ -640,6 +640,7 @@ static coroutine_fn int nbd_co_receive_one_chunk(
> request_ret, qiov, payload, errp);
>
> if (ret < 0) {
> + memset(reply, 0, sizeof *reply);
> s->quit = true;
> } else {
> /* For assert at loop start in nbd_connection_entry */
if (reply) {
*reply = s->reply;
}
either callers can pass in reply==NULL (in which case the memset()
dereferences NULL, oops), or always pass in non-NULL reply (in which
case the null check is dead code).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
