Re: [Qemu-block] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

[Eric Blake]

On 06/13/2016 06:19 AM, Paolo Bonzini wrote:
> 
> 
> On 12/05/2016 00:39, Eric Blake wrote:
>> We have a few bugs in how we handle invalid client commands:
>>
>> - A client can send an NBD_CMD_DISC where from + len overflows,
>> convincing us to reply with an error and stay connected, even
>> though the protocol requires us to silently disconnect. Fix by
>> hoisting the special case sooner.
>>

> It's simpler to always set req->complete.  Putting everything together:
> 
> diff --git a/nbd/server.c b/nbd/server.c

> @@ -1213,12 +1218,9 @@ static void nbd_trip(void *opaque)
>          LOG("invalid request type (%" PRIu32 ") received", request.type);
>          reply.error = EINVAL;
>      error_reply:
> -        /* We must disconnect after replying with an error to
> -         * NBD_CMD_READ, since we choose not to send bogus filler
> -         * data; likewise after NBD_CMD_WRITE if we did not read the
> -         * payload. */
> -        if (nbd_co_send_reply(req, &reply, 0) < 0 || command == NBD_CMD_READ 
> ||
> -            (command == NBD_CMD_WRITE && !req->complete)) {
> +        /* We must disconnect after NBD_CMD_WRITE if we did not
> +         * read the payload. */
> +        if (nbd_co_send_reply(req, &reply, 0) < 0 || !req->complete)) {

This doesn't even compile (too many ')').  I assume you'll fix that
before your actual pull request goes out.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature