qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, qemu-block@nongnu.org,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [Qemu-devel] [PATCH for-2.6?] nbd: Don't mishandle unaligned client requests
Date: Thu, 21 Apr 2016 08:42:30 -0600	[thread overview]
Message-ID: <1461249750-31928-1-git-send-email-eblake@redhat.com> (raw)

The NBD protocol does not (yet) force any alignment constraints
on clients.  Even though qemu NBD clients always send requests
that are aligned to 512 bytes, we must be prepared for non-qemu
clients that don't care about alignment (even if it means they
are less efficient).  Our use of blk_read() and blk_write() was
silently operating on the wrong file offsets when the client
made an unaligned request, corrupting the client's data (but
as the client already has control over the file we are serving,
I don't think it is a security hole, per se, just a data
corruption bug).

Note that in the case of NBD_CMD_READ, an unaligned length could
cause us to return up to 511 bytes of uninitialized trailing
garbage from blk_try_blockalign() - hopefully nothing sensitive
from the heap's prior usage is ever leaked in that manner.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

It's late for 2.6, but as a data corruption bug fix, I think
it's worth having if there is still time.

 nbd/server.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index a13a691..2184c64 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1091,9 +1091,8 @@ static void nbd_trip(void *opaque)
             }
         }

-        ret = blk_read(exp->blk,
-                       (request.from + exp->dev_offset) / BDRV_SECTOR_SIZE,
-                       req->data, request.len / BDRV_SECTOR_SIZE);
+        ret = blk_pread(exp->blk, request.from + exp->dev_offset,
+                        req->data, request.len);
         if (ret < 0) {
             LOG("reading from file failed");
             reply.error = -ret;
@@ -1115,9 +1114,8 @@ static void nbd_trip(void *opaque)

         TRACE("Writing to device");

-        ret = blk_write(exp->blk,
-                        (request.from + exp->dev_offset) / BDRV_SECTOR_SIZE,
-                        req->data, request.len / BDRV_SECTOR_SIZE);
+        ret = blk_pwrite(exp->blk, request.from + exp->dev_offset,
+                        req->data, request.len);
         if (ret < 0) {
             LOG("writing to file failed");
             reply.error = -ret;
-- 
2.5.5

             reply	other threads:[~2016-04-21 14:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-21 14:42 Eric Blake [this message]
2016-04-21 16:28 ` [Qemu-devel] [PATCH for-2.6?] nbd: Don't mishandle unaligned client requests Peter Maydell
2016-04-22  7:03   ` Kevin Wolf
2016-04-22  9:29     ` Peter Maydell
2016-04-22 10:19       ` Kevin Wolf
2016-04-22 11:19         ` Peter Maydell
2016-04-22  7:22 ` Fam Zheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1461249750-31928-1-git-send-email-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).