Re: [Qemu-block] [PATCH v2 for-2.11] nbd/server: Fix error reporting for bad requests

[Eric Blake]

On 11/16/2017 02:52 AM, Vladimir Sementsov-Ogievskiy wrote:

>>       if (request->type == NBD_CMD_READ || request->type ==
>> NBD_CMD_WRITE) {
>>           if (request->len > NBD_MAX_BUFFER_SIZE) {
>>               error_setg(errp, "len (%" PRIu32" ) is larger than max
>> len (%u)",
> 
> related idea here: if request->len > NBD_MAX_BUFFER_SIZE or if we failed
> to allocate buffer in following if,
> we can call nbd_drop to read CMD_WRITE payload and set req->complete =
> true;, to keep connection in this
> cases.
> 
> However, it may be done later.

On the other hand, if request->len is too huge (it can be up to 4G,
where we only want 32M at the most), then we really don't want to waste
time on the nbd_drop() call.  That's why req->complete exists in the
first place.  You are right that we could nbd_drop() a padded size (for
example, I just patched nbdkit to gracefully skip up to twice the
maximum block size, and only drop the connection when it exceeds
NBD_MAX_BUFFER_SIZE*2), but again, it's all in the
quality-of-implementation area (a client sending that much data is
already in denial-of-service territory, so we are okay dropping the
connection).  So I'm not worried about any further tweaks to this code
for 2.11.


> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

Thanks; this patch is now on my queue for 2.11-rc2.

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

signature.asc
Description: OpenPGP digital signature