10:08 p.m.
Now instead of validating just the client's request, we are validating
that all filters are passing valid requests on down.
In protocol.c, we were able to assert that our computation of eflags
populated all of the flags, and thus calls such as backend_can_write
would not fail; however, with filters, keeping those assertions mean
the burden is now on the filter to avoid calling into next_ops->FOO
without first priming the cache of next_ops->can_FOO. An audit of
existing filters did not find any more culprits.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
server/backend.c | 94 ++++++++++++++++++++++++++++++++
server/protocol.c | 134 ++++++++++------------------------------------
2 files changed, 121 insertions(+), 107 deletions(-)
diff --git a/server/backend.c b/server/backend.c
index e785a824..cd678f96 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -124,6 +124,43 @@ backend_unload (struct backend *b, void (*unload) (void))
free (b->name);
}
+static bool
+writes_blocked (struct backend *b, struct connection *conn, const char *cmd,
+ uint32_t flags)
+{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
+ assert (h->can_write >= 0);
+ if (h->can_write == 0) {
+ nbdkit_error ("invalid request: %s: write request on readonly connection",
+ cmd);
+ return true;
+ }
+ if (flags & NBDKIT_FLAG_FUA) {
+ assert (h->can_fua >= 0);
+ if (h->can_fua == 0) {
+ nbdkit_error ("invalid request: %s: FUA flag not supported", cmd);
+ return true;
+ }
+ }
+ return false;
+}
+
+static bool
+invalid_range (struct backend *b, struct connection *conn, const char *cmd,
+ uint64_t offset, uint32_t count)
+{
+ struct b_conn_handle *h = &conn->handles[b->i];
+
+ assert (h->exportsize >= 0);
+ if (!count || offset > h->exportsize || offset + count > h->exportsize) {
+ nbdkit_error ("invalid request: %s: offset and count are out of range: "
+ "offset=%" PRIu64 " count=%" PRIu32, cmd, offset,
count);
+ return true;
+ }
+ return false;
+}
+
void
backend_set_handle (struct backend *b, struct connection *conn, void *handle)
{
@@ -283,6 +320,10 @@ backend_pread (struct backend *b, struct connection *conn,
debug ("%s: pread count=%" PRIu32 " offset=%" PRIu64,
b->name, count, offset);
+ if (invalid_range (b, conn, "pread", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
r = b->pread (b, conn, buf, count, offset, flags, err);
if (r == -1)
assert (*err);
@@ -300,6 +341,14 @@ backend_pwrite (struct backend *b, struct connection *conn,
debug ("%s: pwrite count=%" PRIu32 " offset=%" PRIu64 "
fua=%d",
b->name, count, offset, !!(flags & NBDKIT_FLAG_FUA));
+ if (writes_blocked (b, conn, "pwrite", flags)) {
+ *err = EROFS;
+ return -1;
+ }
+ if (invalid_range (b, conn, "pwrite", offset, count)) {
+ *err = ENOSPC;
+ return -1;
+ }
r = b->pwrite (b, conn, buf, count, offset, flags, err);
if (r == -1)
assert (*err);
@@ -310,11 +359,18 @@ int
backend_flush (struct backend *b, struct connection *conn,
uint32_t flags, int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (flags == 0);
debug ("%s: flush", b->name);
+ assert (h->can_flush >= 0);
+ if (h->can_flush == 0) {
+ nbdkit_error ("invalid request: flush operation not supported");
+ *err = EINVAL;
+ return -1;
+ }
r = b->flush (b, conn, flags, err);
if (r == -1)
assert (*err);
@@ -326,12 +382,27 @@ backend_trim (struct backend *b, struct connection *conn,
uint32_t count, uint64_t offset, uint32_t flags,
int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (flags == 0);
debug ("%s: trim count=%" PRIu32 " offset=%" PRIu64 "
fua=%d",
b->name, count, offset, !!(flags & NBDKIT_FLAG_FUA));
+ if (writes_blocked (b, conn, "trim", flags)) {
+ *err = EROFS;
+ return -1;
+ }
+ if (invalid_range (b, conn, "trim", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
+ assert (h->can_trim >= 0);
+ if (h->can_trim == 0) {
+ nbdkit_error ("invalid request: trim operation not supported");
+ *err = EINVAL;
+ return -1;
+ }
r = b->trim (b, conn, count, offset, flags, err);
if (r == -1)
assert (*err);
@@ -343,6 +414,7 @@ backend_zero (struct backend *b, struct connection *conn,
uint32_t count, uint64_t offset, uint32_t flags,
int *err)
{
+ struct b_conn_handle *h = &conn->handles[b->i];
int r;
assert (!(flags & ~(NBDKIT_FLAG_MAY_TRIM | NBDKIT_FLAG_FUA)));
@@ -350,6 +422,20 @@ backend_zero (struct backend *b, struct connection *conn,
b->name, count, offset, !!(flags & NBDKIT_FLAG_MAY_TRIM),
!!(flags & NBDKIT_FLAG_FUA));
+ if (writes_blocked (b, conn, "zero", flags)) {
+ *err = EROFS;
+ return -1;
+ }
+ if (invalid_range (b, conn, "zero", offset, count)) {
+ *err = ENOSPC;
+ return -1;
+ }
+ assert (h->can_zero >= 0);
+ if (h->can_zero == NBDKIT_ZERO_NONE) {
+ nbdkit_error ("invalid request: write zeroes operation not supported");
+ *err = EINVAL;
+ return -1;
+ }
r = b->zero (b, conn, count, offset, flags, err);
if (r == -1)
assert (*err && *err != ENOTSUP && *err != EOPNOTSUPP);
@@ -368,6 +454,10 @@ backend_extents (struct backend *b, struct connection *conn,
debug ("%s: extents count=%" PRIu32 " offset=%" PRIu64 "
req_one=%d",
b->name, count, offset, !!(flags & NBDKIT_FLAG_REQ_ONE));
+ if (invalid_range (b, conn, "extents", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
assert (h->can_extents >= 0);
if (h->can_extents == 0) {
/* By default it is safe assume that everything in the range is
@@ -396,6 +486,10 @@ backend_cache (struct backend *b, struct connection *conn,
debug ("%s: cache count=%" PRIu32 " offset=%" PRIu64,
b->name, count, offset);
+ if (invalid_range (b, conn, "cache", offset, count)) {
+ *err = EINVAL;
+ return -1;
+ }
assert (h->can_cache >= 0);
if (h->can_cache == NBDKIT_CACHE_NONE) {
nbdkit_error ("invalid request: cache operation not supported");
diff --git a/server/protocol.c b/server/protocol.c
index 03862f5f..db40cca3 100644
--- a/server/protocol.c
+++ b/server/protocol.c
@@ -47,50 +47,43 @@
#include "minmax.h"
#include "protocol.h"
-static bool
-valid_range (struct connection *conn, uint64_t offset, uint32_t count)
-{
- uint64_t exportsize = backend_get_size (backend, conn);
-
- assert (exportsize <= INT64_MAX); /* Guaranteed by negotiation phase */
- return count > 0 && offset <= exportsize && offset + count <=
exportsize;
-}
-
static bool
validate_request (struct connection *conn,
uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
uint32_t *error)
{
- int r;
-
- /* Readonly connection? */
- if (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM ||
- cmd == NBD_CMD_WRITE_ZEROES) {
- r = backend_can_write (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: write request on readonly
connection",
- name_of_nbd_cmd (cmd));
- *error = EROFS;
- return false;
- }
- }
-
- /* Validate cmd, offset, count. */
+ /* Validate command arguments (more checks will be done later in backend) */
switch (cmd) {
- case NBD_CMD_READ:
case NBD_CMD_CACHE:
- case NBD_CMD_WRITE:
case NBD_CMD_TRIM:
case NBD_CMD_WRITE_ZEROES:
+ break;
+
+ case NBD_CMD_READ:
+ case NBD_CMD_WRITE:
+ /* Refuse over-large read and write requests. */
+ if (count > MAX_REQUEST_SIZE) {
+ nbdkit_error ("invalid request: %s: data request is too large (%" PRIu32
+ " > %d)",
+ name_of_nbd_cmd (cmd), count, MAX_REQUEST_SIZE);
+ *error = ENOMEM;
+ return false;
+ }
+ break;
+
case NBD_CMD_BLOCK_STATUS:
- if (!valid_range (conn, offset, count)) {
- /* XXX Allow writes to extend the disk? */
- nbdkit_error ("invalid request: %s: offset and count are out of range: "
- "offset=%" PRIu64 " count=%" PRIu32,
- name_of_nbd_cmd (cmd), offset, count);
- *error = (cmd == NBD_CMD_WRITE ||
- cmd == NBD_CMD_WRITE_ZEROES) ? ENOSPC : EINVAL;
+ if (!conn->structured_replies) {
+ nbdkit_error ("invalid request: "
+ "%s: structured replies was not negotiated",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
+ return false;
+ }
+ if (!conn->meta_context_base_allocation) {
+ nbdkit_error ("invalid request: "
+ "%s: base:allocation was not negotiated",
+ name_of_nbd_cmd (cmd));
+ *error = EINVAL;
return false;
}
break;
@@ -144,79 +137,6 @@ validate_request (struct connection *conn,
*error = EINVAL;
return false;
}
- if (flags & NBD_CMD_FLAG_FUA) {
- r = backend_can_fua (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: FUA flag not supported");
- *error = EINVAL;
- return false;
- }
- }
-
- /* Refuse over-large read and write requests. */
- if ((cmd == NBD_CMD_WRITE || cmd == NBD_CMD_READ) &&
- count > MAX_REQUEST_SIZE) {
- nbdkit_error ("invalid request: %s: data request is too large (%" PRIu32
- " > %d)",
- name_of_nbd_cmd (cmd), count, MAX_REQUEST_SIZE);
- *error = ENOMEM;
- return false;
- }
-
- /* Flush allowed? */
- if (cmd == NBD_CMD_FLUSH) {
- r = backend_can_flush (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: flush operation not supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
-
- /* Trim allowed? */
- if (cmd == NBD_CMD_TRIM) {
- r = backend_can_trim (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: trim operation not supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
-
- /* Zero allowed? */
- if (cmd == NBD_CMD_WRITE_ZEROES) {
- r = backend_can_zero (backend, conn);
- assert (r >= 0); /* Guaranteed by eflags computation */
- if (!r) {
- nbdkit_error ("invalid request: %s: write zeroes operation not
supported",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
-
- /* Block status allowed? */
- if (cmd == NBD_CMD_BLOCK_STATUS) {
- if (!conn->structured_replies) {
- nbdkit_error ("invalid request: "
- "%s: structured replies was not negotiated",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- if (!conn->meta_context_base_allocation) {
- nbdkit_error ("invalid request: "
- "%s: base:allocation was not negotiated",
- name_of_nbd_cmd (cmd));
- *error = EINVAL;
- return false;
- }
- }
return true; /* Command validates. */
}
--
2.21.0