Commit df7b97ff authored by Eric Blake's avatar Eric Blake Committed by Peter Maydell
Browse files

nbd: Don't mishandle unaligned client requests



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: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
Reviewed-by: default avatarFam Zheng <famz@redhat.com>
Tested-by: default avatarKevin Wolf <kwolf@redhat.com>
Message-id: 1461249750-31928-1-git-send-email-eblake@redhat.com
Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
parent 8d0d9b9f
Loading
Loading
Loading
Loading
+4 −6
Original line number Diff line number Diff line
@@ -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;