4:16 p.m.
We had several inconsistencies from the NBD spec when diagnosing
bad client messages:
- FLUSH is not generally forbidden on a read-only export (so failing
with EPERM is wrong) [meanwhile, if we don't advertise flush because
plugin_can_flush() fails, then rejecting with EINVAL is still okay]
- returning EPERM (aka EROFS) for read-only exports should probably
take precedence over anything else
- out-of-bounds WRITE and WRITE_ZEROES should fail with ENOSPC,
not EIO
- out-of-bounds READ and TRIM should fail with EINVAL, not EIO
We also had dead code: valid_range() and validate_request() cannot
return -1. Make these functions return bool instead. And finally,
fix a comment typo.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/connections.c | 53 ++++++++++++++++++++++++-----------------------------
1 file changed, 24 insertions(+), 29 deletions(-)
diff --git a/src/connections.c b/src/connections.c
index 8dc1925..264037d 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -666,7 +666,7 @@ negotiate_handshake (struct connection *conn)
return r;
}
-static int
+static bool
valid_range (struct connection *conn, uint64_t offset, uint32_t count)
{
uint64_t exportsize = conn->exportsize;
@@ -674,27 +674,34 @@ valid_range (struct connection *conn, uint64_t offset, uint32_t
count)
return count > 0 && offset <= exportsize && offset + count <=
exportsize;
}
-static int
+static bool
validate_request (struct connection *conn,
uint32_t cmd, uint32_t flags, uint64_t offset, uint32_t count,
uint32_t *error)
{
int r;
+ /* Readonly connection? */
+ if (conn->readonly &&
+ (cmd == NBD_CMD_WRITE ||
+ cmd == NBD_CMD_TRIM || cmd == NBD_CMD_WRITE_ZEROES)) {
+ nbdkit_error ("invalid request: write request on readonly connection");
+ *error = EROFS;
+ return false;
+ }
+
/* Validate cmd, offset, count. */
switch (cmd) {
case NBD_CMD_READ:
case NBD_CMD_WRITE:
case NBD_CMD_TRIM:
case NBD_CMD_WRITE_ZEROES:
- r = valid_range (conn, offset, count);
- if (r == -1)
- return -1;
- if (r == 0) {
+ if (!valid_range (conn, offset, count)) {
/* XXX Allow writes to extend the disk? */
nbdkit_error ("invalid request: offset and length are out of range");
- *error = EIO;
- return 0;
+ *error = (cmd == NBD_CMD_WRITE ||
+ cmd == NBD_CMD_WRITE_ZEROES) ? ENOSPC : EINVAL;
+ return false;
}
break;
@@ -702,7 +709,7 @@ validate_request (struct connection *conn,
if (offset != 0 || count != 0) {
nbdkit_error ("invalid flush request: expecting offset and length ==
0");
*error = EINVAL;
- return 0;
+ return false;
}
break;
@@ -710,20 +717,20 @@ validate_request (struct connection *conn,
nbdkit_error ("invalid request: unknown command (%" PRIu32 ")
ignored",
cmd);
*error = EINVAL;
- return 0;
+ return false;
}
/* Validate flags */
if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE)) {
nbdkit_error ("invalid request: unknown flag (0x%x)", flags);
*error = EINVAL;
- return 0;
+ return false;
}
if ((flags & NBD_CMD_FLAG_NO_HOLE) &&
cmd != NBD_CMD_WRITE_ZEROES) {
nbdkit_error ("invalid request: NO_HOLE flag needs WRITE_ZEROES request");
*error = EINVAL;
- return 0;
+ return false;
}
/* Refuse over-large read and write requests. */
@@ -732,33 +739,24 @@ validate_request (struct connection *conn,
nbdkit_error ("invalid request: data request is too large (%" PRIu32
" > %d)", count, MAX_REQUEST_SIZE);
*error = ENOMEM;
- return 0;
- }
-
- /* Readonly connection? */
- if (conn->readonly &&
- (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_FLUSH ||
- cmd == NBD_CMD_TRIM || cmd == NBD_CMD_WRITE_ZEROES)) {
- nbdkit_error ("invalid request: write request on readonly connection");
- *error = EROFS;
- return 0;
+ return false;
}
/* Flush allowed? */
if (!conn->can_flush && cmd == NBD_CMD_FLUSH) {
nbdkit_error ("invalid request: flush operation not supported");
*error = EINVAL;
- return 0;
+ return false;
}
/* Trim allowed? */
if (!conn->can_trim && cmd == NBD_CMD_TRIM) {
nbdkit_error ("invalid request: trim operation not supported");
*error = EINVAL;
- return 0;
+ return false;
}
- return 1; /* Commands validates. */
+ return true; /* Command validates. */
}
/* Grab the appropriate error value.
@@ -970,10 +968,7 @@ recv_request_send_reply (struct connection *conn)
}
/* Validate the request. */
- r = validate_request (conn, cmd, flags, offset, count, &error);
- if (r == -1)
- return -1;
- if (r == 0) { /* request not valid */
+ if (!validate_request (conn, cmd, flags, offset, count, &error)) {
if (cmd == NBD_CMD_WRITE &&
skip_over_write_buffer (conn->sockin, count) < 0)
return -1;
--
2.13.6