[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entr
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry |
Date: |
Mon, 7 Aug 2017 10:33:08 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 08/07/2017 10:13 AM, Eric Blake wrote:
> On 08/07/2017 07:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 07.08.2017 14:52, Eric Blake wrote:
>>> On 08/04/2017 10:14 AM, Vladimir Sementsov-Ogievskiy wrote:
>>>> Set reply.handle to 0 on error path to prevent normal path of
>>>> nbd_co_receive_reply.
>
> ...and the client is recognizing that the server sent garbage, but then
> proceeds to handle the packet anyway. The ideal reaction is that the
> client should disconnect from the server, rather than handle the packet.
> But because it didn't do that, the client is now unable to receive any
> future messages from the server. Compare the traces of:
>
> followed by a clean disconnect; vs. the buggy server:
>
> address@hidden:nbd_opt_go_success Export is good to go
> address@hidden:nbd_send_request Sending request to server: {
> .from = 0, .len = 1, .handle = 94152262559840, .flags = 0x0, .type = 0
> (read) }
> address@hidden:nbd_receive_reply Got reply: { magic =
> 0x1446698, .error = 0, handle = 94152262559840 }
> invalid magic (got 0x1446698)
> read 1/1 bytes at offset 0
> 1 bytes, 1 ops; 0:00:12.10 (0.082581 bytes/sec and 0.0826 ops/sec)
> address@hidden:nbd_send_request Sending request to server: {
> .from = 0, .len = 0, .handle = 94152262559840, .flags = 0x0, .type = 3
> (flush) }
>
> where the client is now hung. Thankfully, the client is blocked in an
> idle loop (not eating CPU), so I don't know if this counts as the
> ability for a malicious server to cause a denial of service against qemu
> as an NBD client (in general, being unable to read further data from the
> server because client internal state is now botched is not that much
> different from being unable to read further data from the server because
> the client hung up on the invalid server), unless there is some way to
> cause qemu to die from an assertion failure rather than just get stuck.
With just patch 6/17 applied, the client still hangs, but this time with
a different trace:
address@hidden:nbd_opt_go_success Export is good to go
address@hidden:nbd_send_request Sending request to server: {
.from = 0, .len = 1, .handle = 94716063746144, .flags = 0x0, .type = 0
(read) }
address@hidden:nbd_receive_reply Got reply: { magic =
0x1446698, .error = 0, handle = 94716063746144 }
invalid magic (got 0x1446698)
read failed: Input/output error
address@hidden:nbd_send_request Sending request to server: {
.from = 0, .len = 0, .handle = 94716063746144, .flags = 0x0, .type = 3
(flush) }
The client still tried to send a flush request to the server, when it
should REALLY quit talking to the server at all and just disconnect,
because the moment the client recognizes that the server has sent
garbage is the moment that the client can no longer assume that the
server will react correctly to any further commands.
So I don't think your patch is quite correct, even if you have
identified a shortfall in our client code.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [PATCH 03/17] nbd/client: refactor nbd_receive_reply, (continued)
- [Qemu-block] [PATCH 16/17] block/nbd-client: drop reply field from NBDClientSession, Vladimir Sementsov-Ogievskiy, 2017/08/04
- [Qemu-block] [PATCH 17/17] block/nbd-client: always return EIO on and after the first io channel error, Vladimir Sementsov-Ogievskiy, 2017/08/04
- [Qemu-block] [PATCH 10/17] block/nbd-client: move nbd_coroutine_end content into nbd_co_request, Vladimir Sementsov-Ogievskiy, 2017/08/04
- [Qemu-block] [PATCH 13/17] block/nbd-client: refactor NBDClientSession.recv_coroutine, Vladimir Sementsov-Ogievskiy, 2017/08/04
- [Qemu-block] [PATCH 06/17] block/nbd-client: fix nbd_read_reply_entry, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-block] [PATCH 07/17] block/nbd-client: refactor request send/receive, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-block] [PATCH 11/17] block/nbd-client: fix nbd_co_request: set s->reply.handle to 0 on error, Vladimir Sementsov-Ogievskiy, 2017/08/04
[Qemu-block] [PATCH 08/17] block/nbd-client: rename nbd_recv_coroutines_enter_all, Vladimir Sementsov-Ogievskiy, 2017/08/04