Skip to content

Commit

Permalink
nbd: Fix large trim/zero requests
Browse files Browse the repository at this point in the history
Although qemu as NBD client limits requests to <2G, the NBD protocol
allows clients to send requests almost all the way up to 4G.  But
because our block layer is not yet 64-bit clean, we accidentally wrap
such requests into a negative size, and fail with EIO instead of
performing the intended operation.

The bug is visible in modern systems with something as simple as:

$ qemu-img create -f qcow2 /tmp/image.img 5G
$ sudo qemu-nbd --connect=/dev/nbd0 /tmp/image.img
$ sudo blkdiscard /dev/nbd0

or with user-space only:

$ truncate --size=3G file
$ qemu-nbd -f raw file
$ nbdsh -u nbd://localhost:10809 -c 'h.trim(3*1024*1024*1024,0)'

Although both blk_co_pdiscard and blk_pwrite_zeroes currently return 0
on success, this is also a good time to fix our code to a more robust
paradigm that treats all non-negative values as success.

Alas, our iotests do not currently make it easy to add external
dependencies on blkdiscard or nbdsh, so we have to rely on manual
testing for now.

This patch can be reverted when we later improve the overall block
layer to be 64-bit clean, but for now, a minimal fix was deemed less
risky prior to release.

CC: [email protected]
Fixes: 1f4d6d1
Fixes: 1c6c4bb
Fixes: systemd/systemd#16242
Signed-off-by: Eric Blake <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
[eblake: rework success tests to use >=0]
  • Loading branch information
ebblake committed Jul 28, 2020
1 parent 1b242c3 commit 890cbcc
Showing 1 changed file with 23 additions and 5 deletions.
28 changes: 23 additions & 5 deletions nbd/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -2378,8 +2378,17 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
if (request->flags & NBD_CMD_FLAG_FAST_ZERO) {
flags |= BDRV_REQ_NO_FALLBACK;
}
ret = blk_pwrite_zeroes(exp->blk, request->from + exp->dev_offset,
request->len, flags);
ret = 0;
/* FIXME simplify this when blk_pwrite_zeroes switches to 64-bit */
while (ret >= 0 && request->len) {
int align = client->check_align ?: 1;
int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
align));
ret = blk_pwrite_zeroes(exp->blk, request->from + exp->dev_offset,
len, flags);
request->len -= len;
request->from += len;
}
return nbd_send_generic_reply(client, request->handle, ret,
"writing to file failed", errp);

Expand All @@ -2393,9 +2402,18 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
"flush failed", errp);

case NBD_CMD_TRIM:
ret = blk_co_pdiscard(exp->blk, request->from + exp->dev_offset,
request->len);
if (ret == 0 && request->flags & NBD_CMD_FLAG_FUA) {
ret = 0;
/* FIXME simplify this when blk_co_pdiscard switches to 64-bit */
while (ret >= 0 && request->len) {
int align = client->check_align ?: 1;
int len = MIN(request->len, QEMU_ALIGN_DOWN(BDRV_REQUEST_MAX_BYTES,
align));
ret = blk_co_pdiscard(exp->blk, request->from + exp->dev_offset,
len);
request->len -= len;
request->from += len;
}
if (ret >= 0 && request->flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->blk);
}
return nbd_send_generic_reply(client, request->handle, ret,
Expand Down

0 comments on commit 890cbcc

Please sign in to comment.