[Qemu-devel] [PATCH for-2.10 0/2] Fix NBD client after server error

Eric Blake posted 2 patches 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170811023759.26390-1-eblake@redhat.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
block/nbd-client.c |  9 +++++++--
nbd/server.c       | 11 +++++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH for-2.10 0/2] Fix NBD client after server error
Posted by Eric Blake 6 years, 7 months ago
Patch 1 is a much smaller patch than Vladimir's attempt [1] at fixing
the client in the face of a malicious server.

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01501.html

Patch 2 is not to be applied; it is a hack for easily creating a
malicious server; by setting NBD_SERVER_DEBUG to a positive integer,
the server will intentionally send bad magic when it reaches that
many replies.

I tested using:
 NBD_SERVER_DEBUG=1 ./qemu-nbd -f raw -x foo file
coupled with
 qemu-io -c 'r 0 1' -c 'r 0 1' -f raw nbd://localhost:10809/foo

Without the patch, the qemu-io client hangs; with the patch, the
client reports 'read failed: Input/output error' for the first read
(where the bad server was detected) and 'read failed: Broken pipe'
for the second (because the client has already dropped the
connection from the bad server).

I would like this to go in -rc3, but would definitely appreciate
review, as the manipulation of coroutines was tricky for me to
step through in the debugger, and I want to make sure I'm not
leaking any memory or stranding an incomplete coroutine.

Eric Blake (2):
  nbd: Drop connection if broken server is detected
  HACK: define NBD_SERVER_DEBUG to force malicious server

 block/nbd-client.c |  9 +++++++--
 nbd/server.c       | 11 +++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.13.4


Re: [Qemu-devel] [PATCH for-2.10 0/2] Fix NBD client after server error
Posted by Eric Blake 6 years, 7 months ago
On 08/10/2017 09:37 PM, Eric Blake wrote:
> Patch 1 is a much smaller patch than Vladimir's attempt [1] at fixing
> the client in the face of a malicious server.
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg01501.html
> 
> Patch 2 is not to be applied; it is a hack for easily creating a
> malicious server; by setting NBD_SERVER_DEBUG to a positive integer,
> the server will intentionally send bad magic when it reaches that
> many replies.
> 
> I tested using:
>  NBD_SERVER_DEBUG=1 ./qemu-nbd -f raw -x foo file
> coupled with
>  qemu-io -c 'r 0 1' -c 'r 0 1' -f raw nbd://localhost:10809/foo
> 
> Without the patch, the qemu-io client hangs; with the patch, the
> client reports 'read failed: Input/output error' for the first read
> (where the bad server was detected) and 'read failed: Broken pipe'
> for the second (because the client has already dropped the
> connection from the bad server).

I've also confirmed that this is a regression from our 2.8 release
(introduced when 2.9 switched NBD to use coroutines).

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