e7b1948d | 12-Mar-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd: BLOCK_STATUS for standard get_block_status function: server part
Minimal realization: only one extent in server answer is supported.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@vir
nbd: BLOCK_STATUS for standard get_block_status function: server part
Minimal realization: only one extent in server answer is supported.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180312152126.286890-4-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: tweak whitespace, move constant from .h to .c, improve logic of check_meta_export_name, simplify nbd_negotiate_options by doing more in nbd_negotiate_meta_queries] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
12296459 | 12-Mar-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: add nbd_read_opt_name helper
Add helper to read name in format:
uint32 len (<= NBD_MAX_NAME_SIZE) len bytes string (not 0-terminated)
The helper will be reused in following p
nbd/server: add nbd_read_opt_name helper
Add helper to read name in format:
uint32 len (<= NBD_MAX_NAME_SIZE) len bytes string (not 0-terminated)
The helper will be reused in following patch.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180312152126.286890-3-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: grammar fixes, actually check error] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
2e425fd5 | 12-Mar-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: add nbd_opt_invalid helper
NBD_REP_ERR_INVALID is often parameter to nbd_opt_drop and it would be used more in following patches. So, let's add a helper.
Signed-off-by: Vladimir Sements
nbd/server: add nbd_opt_invalid helper
NBD_REP_ERR_INVALID is often parameter to nbd_opt_drop and it would be used more in following patches. So, let's add a helper.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20180312152126.286890-2-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
65529782 | 07-Mar-2018 |
Eric Blake <eblake@redhat.com> |
nbd/server: Honor FUA request on NBD_CMD_TRIM
The NBD spec states that since trim requests can affect disk contents, then they should allow for FUA semantics just like writes for ensuring the disk h
nbd/server: Honor FUA request on NBD_CMD_TRIM
The NBD spec states that since trim requests can affect disk contents, then they should allow for FUA semantics just like writes for ensuring the disk has settled before returning. As bdrv_[co_]pdiscard() does not support a flags argument, we can't pass FUA down the block layer stack, and must therefore emulate it with a flush at the NBD layer.
Note that in all reality, generic well-behaved clients will never send TRIM+FUA (in fact, qemu as a client never does, and we have no intention to plumb flags into bdrv_pdiscard). This is because the NBD protocol states that it is unspecified to READ a trimmed area (you might read stale data, all zeroes, or even random unrelated data) without first rewriting it, and even the experimental BLOCK_STATUS extension states that TRIM need not affect reported status. Thus, in the general case, a client cannot tell the difference between an arbitrary server that ignores TRIM, a server that had a power outage without flushing to disk, and a server that actually affected the disk before returning; so waiting for the trim actions to flush to disk makes little sense. However, for a specific client and server pair, where the client knows the server treats TRIM'd areas as guaranteed reads-zero, waiting for a flush makes sense, hence why the protocol documents that FUA is valid on trim. So, even though the NBD protocol doesn't have a way for the server to advertise what effects (if any) TRIM will actually have, and thus any client that relies on specific effects is probably in error, we can at least support a client that requests TRIM+FUA.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20180307225732.155835-1-eblake@redhat.com>
show more ...
|
6f302e60 | 12-Mar-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: refactor nbd_trip: split out nbd_handle_request
Split out request handling logic.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.1785
nbd/server: refactor nbd_trip: split out nbd_handle_request
Split out request handling logic.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-6-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: touch up blank line placement] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
6a417599 | 08-Mar-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: refactor nbd_trip: cmd_read and generic reply
nbd_trip has difficult logic when sending replies: it tries to use one code path for all replies. It is ok for simple replies, but is not co
nbd/server: refactor nbd_trip: cmd_read and generic reply
nbd_trip has difficult logic when sending replies: it tries to use one code path for all replies. It is ok for simple replies, but is not comfortable for structured replies. Also, two types of error (and corresponding messages in local_err) - fatal (leading to disconnect) and not-fatal (just to be sent to the client) are difficult to follow.
To make things a bit clearer, the following is done: - split CMD_READ logic to separate function. It is the most difficult command for now, and it is definitely cramped inside nbd_trip. Also, it is difficult to follow CMD_READ logic, shared between "case NBD_CMD_READ" and "if"s under "reply:" label. - create separate helper function nbd_send_generic_reply() and use it both in new nbd_do_cmd_read and for other commands in nbd_trip instead of common code-path under "reply:" label in nbd_trip. The helper supports an error message, so logic with local_err in nbd_trip is simplified.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-5-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: grammar tweaks and blank line placement] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
a0d7ce20 | 08-Mar-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: fix: check client->closing before sending reply
Since the unchanged code has just set client->recv_coroutine to NULL before calling nbd_client_receive_next_request(), we are spawning a n
nbd/server: fix: check client->closing before sending reply
Since the unchanged code has just set client->recv_coroutine to NULL before calling nbd_client_receive_next_request(), we are spawning a new coroutine unconditionally, but the first thing that coroutine will do is check for client->closing, making it a no-op if we have already detected that the client is going away. Furthermore, for any error other than EIO (where we disconnect, which itself sets client->closing), if the client has already gone away, we'll probably encounter EIO later in the function and attempt disconnect at that point. Logically, as soon as we know the connection is closing, there is no need to try a likely-to-fail a response or spawn a no-op coroutine.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-4-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: squash in further reordering: hoist check before spawning next coroutine, and document rationale in commit message] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
37e02aeb | 08-Mar-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: fix sparse read
In case of io error in nbd_co_send_sparse_read we should not "goto reply:", as it was a fatal error and the common behavior is to disconnect in this case. We should not t
nbd/server: fix sparse read
In case of io error in nbd_co_send_sparse_read we should not "goto reply:", as it was a fatal error and the common behavior is to disconnect in this case. We should not try to send the client an additional error reply, since we already hit a channel-io error on our previous attempt to send one.
Fix this by handling block-status error in nbd_co_send_sparse_read, so nbd_co_send_sparse_read fails only on io error. Then just skip common "reply:" code path in nbd_trip.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180308184636.178534-3-vsementsov@virtuozzo.com> [eblake: grammar tweaks] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
28fb494f | 15-Feb-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/client: fix error messages in nbd_handle_reply_err
1. NBD_REP_ERR_INVALID is not only about length, so, make message more general
2. hex format is not very good: it's hard to read something
nbd/client: fix error messages in nbd_handle_reply_err
1. NBD_REP_ERR_INVALID is not only about length, so, make message more general
2. hex format is not very good: it's hard to read something like "option a (set meta context)", so switch to dec.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <1518702707-7077-6-git-send-email-vsementsov@virtuozzo.com> [eblake: expand scope of patch: ALL uses of nbd_opt_lookup and nbd_rep_lookup are now decimal] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
1d17922a | 10-Jan-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: structurize option reply sending
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171122101958.17065-6-vs
nbd/server: structurize option reply sending
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171122101958.17065-6-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
894e0280 | 10-Jan-2018 |
Eric Blake <eblake@redhat.com> |
nbd/server: Add helper functions for parsing option payload
Rather than making every callsite perform length sanity checks and error reporting, add the helper functions nbd_opt_read() and nbd_opt_dr
nbd/server: Add helper functions for parsing option payload
Rather than making every callsite perform length sanity checks and error reporting, add the helper functions nbd_opt_read() and nbd_opt_drop() that use the length stored in the client struct; also add an assertion that optlen is 0 before any option (ie. any previous option was fully handled), complementing the assertion added in an earlier patch that optlen is 0 after all negotiation completes.
Note that the call in nbd_negotiate_handle_export_name() does not use the new helper (in part because the server cannot reply to NBD_OPT_EXPORT_NAME - it either succeeds or the connection drops).
Based on patches by Vladimir Sementsov-Ogievskiy.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180110230825.18321-6-eblake@redhat.com>
show more ...
|
41f5dfaf | 10-Jan-2018 |
Eric Blake <eblake@redhat.com> |
nbd/server: Add va_list form of nbd_negotiate_send_rep_err()
This will be useful for the next patch.
Based on a patch by Vladimir Sementsov-Ogievskiy
Signed-off-by: Eric Blake <eblake@redhat.com>
nbd/server: Add va_list form of nbd_negotiate_send_rep_err()
This will be useful for the next patch.
Based on a patch by Vladimir Sementsov-Ogievskiy
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180110230825.18321-5-eblake@redhat.com>
show more ...
|
32f158a6 | 10-Jan-2018 |
Eric Blake <eblake@redhat.com> |
nbd/server: Better error for NBD_OPT_EXPORT_NAME failure
When a client abruptly disconnects before we've finished reading the name sent with NBD_OPT_EXPORT_NAME, we are better off logging the failur
nbd/server: Better error for NBD_OPT_EXPORT_NAME failure
When a client abruptly disconnects before we've finished reading the name sent with NBD_OPT_EXPORT_NAME, we are better off logging the failure as EIO (we can't communicate with the client), rather than EINVAL (the client sent bogus data).
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20180110230825.18321-4-eblake@redhat.com>
show more ...
|
0cfae925 | 10-Jan-2018 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd/server: refactor negotiation functions parameters
Instead of passing currently negotiating option and its length to many of negotiation functions let's just store them on NBDClient struct to be
nbd/server: refactor negotiation functions parameters
Instead of passing currently negotiating option and its length to many of negotiation functions let's just store them on NBDClient struct to be state-variables of negotiation phase.
This unifies semantics of negotiation functions and allows tracking changes of remaining option length in future patches.
Asssert that optlen is back to 0 after negotiation (including old-style connections which don't negotiate), although we need more patches before we can assert optlen is 0 between options during negotiation.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171122101958.17065-2-vsementsov@virtuozzo.com> [eblake: rebase, commit message tweak, assert !optlen after negotiation completes] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
420a4e95 | 22-Nov-2017 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd: rename nbd_option and nbd_opt_reply
Rename nbd_option and nbd_opt_reply to NBDOption and NBDOptionReply to correspond to Qemu coding style and other structures here.
Signed-off-by: Vladimir Se
nbd: rename nbd_option and nbd_opt_reply
Rename nbd_option and nbd_opt_reply to NBDOption and NBDOptionReply to correspond to Qemu coding style and other structures here.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20171122101958.17065-5-vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
e2de3256 | 07-Nov-2017 |
Eric Blake <eblake@redhat.com> |
nbd/server: Optimize final chunk of sparse read
If we are careful to handle 0-length read requests correctly, we can optimize our sparse read to send the NBD_REPLY_FLAG_DONE bit on our last OFFSET_D
nbd/server: Optimize final chunk of sparse read
If we are careful to handle 0-length read requests correctly, we can optimize our sparse read to send the NBD_REPLY_FLAG_DONE bit on our last OFFSET_DATA or OFFSET_HOLE chunk rather than needing a separate chunk.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20171107030912.23930-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
51ae4f84 | 22-Nov-2017 |
Eric Blake <eblake@redhat.com> |
nbd/server: CVE-2017-15118 Stack smash on large export name
Introduced in commit f37708f6b8 (2.10). The NBD spec says a client can request export names up to 4096 bytes in length, even though they
nbd/server: CVE-2017-15118 Stack smash on large export name
Introduced in commit f37708f6b8 (2.10). The NBD spec says a client can request export names up to 4096 bytes in length, even though they should not expect success on names longer than 256. However, qemu hard-codes the limit of 256, and fails to filter out a client that probes for a longer name; the result is a stack smash that can potentially give an attacker arbitrary control over the qemu process.
The smash can be easily demonstrated with this client: $ qemu-io f raw nbd://localhost:10809/$(printf %3000d 1 | tr ' ' a)
If the qemu NBD server binary (whether the standalone qemu-nbd, or the builtin server of QMP nbd-server-start) was compiled with -fstack-protector-strong, the ability to exploit the stack smash into arbitrary execution is a lot more difficult (but still theoretically possible to a determined attacker, perhaps in combination with other CVEs). Still, crashing a running qemu (and losing the VM) is bad enough, even if the attacker did not obtain full execution control.
CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|