From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35849) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anXI8-0008PP-RI for qemu-devel@nongnu.org; Tue, 05 Apr 2016 16:09:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anXI4-0002Dc-PZ for qemu-devel@nongnu.org; Tue, 05 Apr 2016 16:09:36 -0400 References: <1459882500-24316-1-git-send-email-alex@alex.org.uk> From: Eric Blake Message-ID: <57041B79.9080707@redhat.com> Date: Tue, 5 Apr 2016 14:09:29 -0600 MIME-Version: 1.0 In-Reply-To: <1459882500-24316-1-git-send-email-alex@alex.org.uk> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3AwOwbx4RqIqqxmQiHkmU0CsbCV1Sou21" Subject: Re: [Qemu-devel] [PATCH for-2.6] Fix NBD unsupported options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Bligh , "Daniel P. Berrange" , Paolo Bonzini , Kevin Wolf , "qemu-devel@nongnu.org" Cc: Wouter Verhelst , qemu-stable This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --3AwOwbx4RqIqqxmQiHkmU0CsbCV1Sou21 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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. >=20 > According to nbd's proto.md, the format for such a reply > should be: >=20 > 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 se= rver > 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 cas= e > of NBD_REP_SERVER) >=20 > 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. :) >=20 > 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: https://github.com/yoe/nbd/blob/66dfb35/doc/proto.md#L1232 > 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 >=20 > Signed-off-by: Alex Bligh > --- > nbd/client.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake Counts as a bug fix, so it should be safe even during qemu hard freeze. Adding qemu-stable in cc. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --3AwOwbx4RqIqqxmQiHkmU0CsbCV1Sou21 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXBBt5AAoJEKeha0olJ0NqgNEH/3z164w9ksqtI3pHXlDiH1WK Z2fJOTnCoZ4GLQwteV23x7RPKbEcdvk19ieOwe2js5KBfKXKpGRS0BaGXD2IlS1V TkrNImSVD73jmymkZFPW2O3tq77He57E7sl907Wa6912H2ghOjFt0Vv/hLGW+2VW X3Z9REQpFnuub2bs+itCIAZciaKaC0TH2nXPctVRzV5YSPAPtlAoBsgPvOs412Ow gEzen2TAVGstHQLRcs0T86CBjW3SpOYVAM9CpX0LoXM0lyMnOGNpG9XVkZCQRMnC 1yL0NwV9VDJbzIXx9wcJKQUG1sz3I91n/IBnqg5/BP8p9kFju5nykwPRZ7rFSAI= =XZMT -----END PGP SIGNATURE----- --3AwOwbx4RqIqqxmQiHkmU0CsbCV1Sou21--