[Qemu-devel] [PATCH 0/2] nbd: changes around allocating buffers for requests

[Qemu-devel] [PATCH 0/2] nbd: changes around allocating buffers for requests

[Paolo Bonzini]

Notably, patch 2 introduces blk_try_blockalign.  If it is acked by
the block layer maintainers I can submit the patch myself for
inclusion.

Paolo

Paolo Bonzini (2):
  nbd: do not check request length except for reads and writes
  nbd: do not exit on failed memory allocation

 block/block-backend.c          |  5 +++++
 include/sysemu/block-backend.h |  1 +
 nbd.c                          | 20 ++++++++++++--------
 3 files changed, 18 insertions(+), 8 deletions(-)

-- 
2.5.0

[Qemu-devel] [PATCH 1/2] nbd: do not check request length except for reads and writes

[Paolo Bonzini]

Only reads and writes need to allocate memory correspondent to the
request length.  Other requests can be sent to the storage without
allocating any memory, and thus any request length is acceptable.

Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/nbd.c b/nbd.c
index b3d9654..e395a16 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1227,13 +1227,6 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque
         goto out;
     }
 
-    if (request->len > NBD_MAX_BUFFER_SIZE) {
-        LOG("len (%u) is larger than max len (%u)",
-            request->len, NBD_MAX_BUFFER_SIZE);
-        rc = -EINVAL;
-        goto out;
-    }
-
     if ((request->from + request->len) < request->from) {
         LOG("integer overflow detected! "
             "you're probably being attacked");
@@ -1245,6 +1238,13 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque
 
     command = request->type & NBD_CMD_MASK_COMMAND;
     if (command == NBD_CMD_READ || command == NBD_CMD_WRITE) {
+        if (request->len > NBD_MAX_BUFFER_SIZE) {
+            LOG("len (%u) is larger than max len (%u)",
+                request->len, NBD_MAX_BUFFER_SIZE);
+            rc = -EINVAL;
+            goto out;
+        }
+
         req->data = blk_blockalign(client->exp->blk, request->len);
     }
     if (command == NBD_CMD_WRITE) {
-- 
2.5.0

[Qemu-devel] [PATCH 2/2] nbd: do not exit on failed memory allocation

[Paolo Bonzini]

The amount of memory allocated in nbd_co_receive_request is driven by the
NBD client (possibly a virtual machine).  Parallel I/O can cause the
server to allocate a large amount of memory; check for failures and
return ENOMEM in that case.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 block/block-backend.c          | 5 +++++
 include/sysemu/block-backend.h | 1 +
 nbd.c                          | 6 +++++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index f41d326..e813759 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1033,6 +1033,11 @@ void blk_set_guest_block_size(BlockBackend *blk, int align)
     blk->guest_block_size = align;
 }
 
+void *blk_try_blockalign(BlockBackend *blk, size_t size)
+{
+    return qemu_try_blockalign(blk ? blk->bs : NULL, size);
+}
+
 void *blk_blockalign(BlockBackend *blk, size_t size)
 {
     return qemu_blockalign(blk ? blk->bs : NULL, size);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index dc24476..1568554 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -148,6 +148,7 @@ int blk_get_flags(BlockBackend *blk);
 int blk_get_max_transfer_length(BlockBackend *blk);
 int blk_get_max_iov(BlockBackend *blk);
 void blk_set_guest_block_size(BlockBackend *blk, int align);
+void *blk_try_blockalign(BlockBackend *blk, size_t size);
 void *blk_blockalign(BlockBackend *blk, size_t size);
 bool blk_op_is_blocked(BlockBackend *blk, BlockOpType op, Error **errp);
 void blk_op_unblock(BlockBackend *blk, BlockOpType op, Error *reason);
diff --git a/nbd.c b/nbd.c
index e395a16..7dc58a9 100644
--- a/nbd.c
+++ b/nbd.c
@@ -1245,7 +1245,11 @@ static ssize_t nbd_co_receive_request(NBDRequest *req, struct nbd_request *reque
             goto out;
         }
 
-        req->data = blk_blockalign(client->exp->blk, request->len);
+        req->data = blk_try_blockalign(client->exp->blk, request->len);
+        if (req->data == NULL) {
+            rc = -ENOMEM;
+            goto out;
+        }
     }
     if (command == NBD_CMD_WRITE) {
         TRACE("Reading %u byte(s)", request->len);
-- 
2.5.0

Re: [Qemu-devel] [PATCH 1/2] nbd: do not check request length except for reads and writes

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

On 07.01.2016 14:44, Paolo Bonzini wrote:
> Only reads and writes need to allocate memory correspondent to the
> request length.  Other requests can be sent to the storage without
> allocating any memory, and thus any request length is acceptable.
> 
> Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com>
> Cc: qemu-block@nongnu.org
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  nbd.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [PATCH 2/2] nbd: do not exit on failed memory allocation

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

On 07.01.2016 14:44, Paolo Bonzini wrote:
> The amount of memory allocated in nbd_co_receive_request is driven by the
> NBD client (possibly a virtual machine).  Parallel I/O can cause the
> server to allocate a large amount of memory; check for failures and
> return ENOMEM in that case.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block/block-backend.c          | 5 +++++
>  include/sysemu/block-backend.h | 1 +
>  nbd.c                          | 6 +++++-
>  3 files changed, 11 insertions(+), 1 deletion(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 0/2] nbd: changes around allocating buffers for requests

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

On Thu, Jan 07, 2016 at 02:44:24PM +0100, Paolo Bonzini wrote:
> Notably, patch 2 introduces blk_try_blockalign.  If it is acked by
> the block layer maintainers I can submit the patch myself for
> inclusion.
> 
> Paolo
> 
> Paolo Bonzini (2):
>   nbd: do not check request length except for reads and writes
>   nbd: do not exit on failed memory allocation
> 
>  block/block-backend.c          |  5 +++++
>  include/sysemu/block-backend.h |  1 +
>  nbd.c                          | 20 ++++++++++++--------
>  3 files changed, 18 insertions(+), 8 deletions(-)
> 
> -- 
> 2.5.0
> 
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]