Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-14 Thread Paolo Bonzini
On 14/06/2016 17:59, Alex Bligh wrote: > >> On 14 Jun 2016, at 16:11, Paolo Bonzini wrote: >> >>> To illustrate the problem, look consider what qemu itself would do as >>> a server if it can't buffer the entire read issued to it. >> >> Return ENOMEM? > > Well OK, qemu

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-14 Thread Eric Blake
On 06/13/2016 06:19 AM, Paolo Bonzini wrote: > > > On 12/05/2016 00:39, Eric Blake wrote: >> We have a few bugs in how we handle invalid client commands: >> >> - A client can send an NBD_CMD_DISC where from + len overflows, >> convincing us to reply with an error and stay connected, even >>

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-14 Thread Alex Bligh
> On 14 Jun 2016, at 16:11, Paolo Bonzini wrote: > >> To illustrate the problem, look consider what qemu itself would do as >> a server if it can't buffer the entire read issued to it. > > Return ENOMEM? Well OK, qemu then 'works' on the basis it breaks another part of

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-14 Thread Alex Bligh
On 14 Jun 2016, at 14:32, Paolo Bonzini wrote: > > On 13/06/2016 23:41, Alex Bligh wrote: >> That's one of the reasons that there is a proposal to add >> STRUCTURED_READ to the spec (although I still haven't had time to >> implement that for qemu), so that we have a newer

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-14 Thread Paolo Bonzini
On 14/06/2016 17:02, Alex Bligh wrote: > > On 14 Jun 2016, at 14:32, Paolo Bonzini wrote: > >> >> On 13/06/2016 23:41, Alex Bligh wrote: >>> That's one of the reasons that there is a proposal to add >>> STRUCTURED_READ to the spec (although I still haven't had time to >>>

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-14 Thread Paolo Bonzini
On 13/06/2016 23:41, Alex Bligh wrote: > That's one of the reasons that there is a proposal to add > STRUCTURED_READ to the spec (although I still haven't had time to > implement that for qemu), so that we have a newer approach that allows > for proper error handling without ambiguity on whether

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Alex Bligh
On 13 Jun 2016, at 13:25, Eric Blake wrote: > On 06/13/2016 06:10 AM, Paolo Bonzini wrote: >> >> >> On 12/05/2016 00:39, Eric Blake wrote: >>> - If we report an error to NBD_CMD_READ, we are not writing out >>> any data payload; but the protocol says that a client can

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Eric Blake
On 06/13/2016 06:19 AM, Paolo Bonzini wrote: >> +/* Sanity checks, part 2. */ >> +if (request->from + request->len > client->exp->size) { >> +LOG("operation past EOF; From: %" PRIu64 ", Len: %" PRIu32 >> +", Size: %" PRIu64, request->from, request->len, >> +

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Eric Blake
[adding nbd list] On 06/13/2016 06:10 AM, Paolo Bonzini wrote: > > > On 12/05/2016 00:39, Eric Blake wrote: >> - If we report an error to NBD_CMD_READ, we are not writing out >> any data payload; but the protocol says that a client can expect >> to read the payload no matter what (and must

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Paolo Bonzini
On 12/05/2016 00:39, Eric Blake wrote: > We have a few bugs in how we handle invalid client commands: > > - A client can send an NBD_CMD_DISC where from + len overflows, > convincing us to reply with an error and stay connected, even > though the protocol requires us to silently disconnect. Fix

Re: [Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-06-13 Thread Paolo Bonzini
On 12/05/2016 00:39, Eric Blake wrote: > - If we report an error to NBD_CMD_READ, we are not writing out > any data payload; but the protocol says that a client can expect > to read the payload no matter what (and must instead ignore it), > which means the client will start reading our next

[Qemu-devel] [PATCH v4 04/11] nbd: Improve server handling of bogus commands

2016-05-11 Thread Eric Blake
We have a few bugs in how we handle invalid client commands: - A client can send an NBD_CMD_DISC where from + len overflows, convincing us to reply with an error and stay connected, even though the protocol requires us to silently disconnect. Fix by hoisting the special case sooner. - A client