[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH for-2.6] Fix NBD unsupported options

From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH for-2.6] Fix NBD unsupported options
Date: Tue, 5 Apr 2016 14:09:29 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1

On 04/05/2016 12:55 PM, Alex Bligh wrote:
> nbd-client.c currently fails to handle unsupported options properly.
> If during option haggling the server finds an option that is
> unsupported, it returns an NBD_REP_ERR_UNSUP reply.
> According to nbd's proto.md, the format for such a reply
> should be:
>   S: 64 bits, 0x3e889045565a9 (magic number for replies)
>   S: 32 bits, the option as sent by the client to which this is a reply
>   S: 32 bits, reply type (e.g., NBD_REP_ACK for successful completion,
>      or NBD_REP_ERR_UNSUP to mark use of an option not known by this server
>   S: 32 bits, length of the reply. This may be zero for some replies,
>      in which case the next field is not sent
>   S: any data as required by the reply (e.g., an export name in the case
>      of NBD_REP_SERVER)
> However, in nbd-client.c, the reply type was being read, and if it
> contained an error, it was bailing out and issuing the next option
> request without first reading the length. This meant that the
> next option / handshake read had an extra 4 bytes of data in it.
> In practice, this makes Qemu incompatible with servers that do not
> support NBD_OPT_LIST.

Or any other option that we are adding; I probably would have hit the
same bug in my work on NBD_OPT_STRUCTURED_REPLY. :)

> To verify this isn't an error in the specification or my reading of
> it, replies are sent by the reference implementation here:
>  https://github.com/yoe/nbd/blob/master/nbd-server.c#L1232

Not a stable link (master moves), but accurate for today.  A stable link
would also peg a particular commit id (7 or so hex digits would be fine:

> and as is evident it always sends a 'datasize' (aka length) 32 bit
> word. Unsupported elements are replied to here:
>  https://github.com/yoe/nbd/blob/master/nbd-server.c#L1371
> Signed-off-by: Alex Bligh <address@hidden>
> ---
>  nbd/client.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

Reviewed-by: Eric Blake <address@hidden>

Counts as a bug fix, so it should be safe even during qemu hard freeze.
 Adding qemu-stable in cc.

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

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]