On Thu, 04/21 08:42, Eric Blake wrote:
> 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 <ebl...@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);
Indentation is one column off, but can be ignored or fixed when applying.
Reviewed-by: Fam Zheng <f...@redhat.com>
> if (ret < 0) {
> LOG("writing to file failed");
> reply.error = -ret;
> --
> 2.5.5
>
>
