Re: [PATCH for-7.1 1/8] nbd: actually implement reply_possible safeguard

[Eric Blake]

On Tue, Apr 12, 2022 at 09:41:57PM +0200, Paolo Bonzini wrote:
> The .reply_possible field of s->requests is never set to false.  This is
> not a big problem as it is only a safeguard to detect protocol errors,
> but fix it anyway.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/nbd.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index 567872ac53..6a5e410e5f 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -454,15 +454,16 @@ static coroutine_fn int 
> nbd_receive_replies(BDRVNBDState *s, uint64_t handle)
>              nbd_channel_error(s, -EINVAL);
>              return -EINVAL;
>          }
> -        if (s->reply.handle == handle) {
> -            /* We are done */
> -            return 0;
> -        }
>          ind2 = HANDLE_TO_INDEX(s, s->reply.handle);
>          if (ind2 >= MAX_NBD_REQUESTS || !s->requests[ind2].reply_possible) {
>              nbd_channel_error(s, -EINVAL);
>              return -EINVAL;
>          }
> +        s->requests[ind2].reply_possible = 
> nbd_reply_is_structured(&s->reply);

If the reply is simple (not structured), then we expect no further
replies, so this sets things to false.  But if the reply is
structured, the answer depends on NBD_REPLY_FLAG_DONE, as in:

s->requests[ind2].reply_possible =
  nbd_reply_is_structured(&s->reply) &&
  (s->reply.structured.flags & NBD_REPLY_FLAG_DONE);

> +        if (s->reply.handle == handle) {
> +            /* We are done */
> +            return 0;
> +        }
>          nbd_recv_coroutine_wake_one(&s->requests[ind2]);
>      }
>  }

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org