From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anq6F-00008B-Hr for qemu-devel@nongnu.org; Wed, 06 Apr 2016 12:14:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1anq6A-0002J9-Jl for qemu-devel@nongnu.org; Wed, 06 Apr 2016 12:14:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60631) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1anq6A-0002In-CP for qemu-devel@nongnu.org; Wed, 06 Apr 2016 12:14:30 -0400 References: <1459882500-24316-1-git-send-email-alex@alex.org.uk> From: Eric Blake Message-ID: <570535E4.8020108@redhat.com> Date: Wed, 6 Apr 2016 10:14:28 -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="QlAWEHPCxRQBmOWkuk71QHMrArrpMpHm8" Subject: Re: [Qemu-devel] [PATCH] 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 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --QlAWEHPCxRQBmOWkuk71QHMrArrpMpHm8 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) I just re-reviewed this patch. While what you have is a strict improvement, it is still buggy for a server that sends a UTF-8 message explaining the error: > @@ -151,6 +145,13 @@ static int nbd_receive_list(QIOChannel *ioc, char = **name, Error **errp) > } > len =3D be32_to_cpu(len); > =20 > + if (type =3D=3D NBD_REP_ERR_UNSUP) { > + return 0; > + } That is, if len > 0, we are still leaving the server's UTF-8 message on the wire, but returning early, which will still corrupt the next attempt to send another option. Paolo, can you revert this off your queue, and wait for me to send a v2 that fixes the situation in a cleaner manner? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --QlAWEHPCxRQBmOWkuk71QHMrArrpMpHm8 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/ iQEcBAEBCAAGBQJXBTXlAAoJEKeha0olJ0NqlycH/1KaA33emRm9uijuP5LumENe VxIR3zPVb93G/3lG5Qkw9TX2wcOM/ecVXCRbLtQO2fvQmZCHLJtgygpxhpWu4P3R C6GFErC5i8iyVJnLCaMrkDMFhIwE+Kgb8pwokBDTWto//fsrFexZulKxoJFiWOzp vIOmjmDz6J3VoZI0QFF7IO+C/RFfSltzPNoLhY1C4U5EtHrui5oK3hS4IIcjVokm 2hr02+SLLj098LtMC8pYCO3naXR3Rg9rZY7t98HB+JJE4GWsqgQKO4PMEyFZeGFX J6qclnSWs5fmJeezEKo5Sm5zKSfnxcUHmyJSMvEaipKnuQPh+5E3v4NVa2ebSbU= =Beo4 -----END PGP SIGNATURE----- --QlAWEHPCxRQBmOWkuk71QHMrArrpMpHm8--