search for: validate_request

...NVAL 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@redhat.com> --- src/connections.c | 53 ++++++++++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/sr...

...f117d42..466be7c 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -36,6 +36,7 @@ #include <stdio.h> #include <stdlib.h> #include <stdint.h> +#include <stdbool.h> #include <inttypes.h> #include <string.h> #include <unistd.h> @@ -78,6 +79,7 @@ validate_request (struct connection *conn, case NBD_CMD_WRITE: case NBD_CMD_TRIM: case NBD_CMD_WRITE_ZEROES: + 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 ou...

...f117d42..c713e12 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -36,6 +36,7 @@ #include <stdio.h> #include <stdlib.h> #include <stdint.h> +#include <stdbool.h> #include <inttypes.h> #include <string.h> #include <unistd.h> @@ -78,6 +79,7 @@ validate_request (struct connection *conn, case NBD_CMD_WRITE: case NBD_CMD_TRIM: case NBD_CMD_WRITE_ZEROES: + 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 ou...

...if (fl) conn->can_extents = true; + if (conn->structured_replies) + eflags |= NBD_FLAG_SEND_DF; + *flags = eflags; return 0; } diff --git a/server/protocol.c b/server/protocol.c index 383938f..d94cd19 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -110,7 +110,7 @@ validate_request (struct connection *conn, /* Validate flags */ if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE | - NBD_CMD_FLAG_REQ_ONE)) { + NBD_CMD_FLAG_DF | NBD_CMD_FLAG_REQ_ONE)) { nbdkit_error ("invalid request: unknown flag (0x%x)", flags); *...

...xtern void nbdkit_error (const char *msg, ...) __attribute__((format (printf, 1, 2))); extern void nbdkit_verror (const char *msg, va_list args); diff --git a/src/connections.c b/src/connections.c index c0f0567..83c863e 100644 --- a/src/connections.c +++ b/src/connections.c @@ -602,6 +602,19 @@ validate_request (struct connection *conn, return 1; /* Commands validates. */ } +/* Grab the appropriate error value. + */ +static int +_get_error (void) +{ + int err = errno; + int ret = tls_get_error (); + + if (!ret) + ret = err ? err : EIO; + return ret; +} + /* This is called...

...if (fl == -1) return -1; - if (fl) - conn->can_extents = true; if (conn->structured_replies) eflags |= NBD_FLAG_SEND_DF; diff --git a/server/protocol.c b/server/protocol.c index 06f1ee15..0ecf0b5c 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -64,14 +64,19 @@ 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 (conn->readonly && - (cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM || - cmd == NBD_...

The upstream protocol recently promoted NBD_CMD_WRITE_ZEROES from experimental to a documented extension. Exposing support for this allows plugin writers to create sparse files when driven by a client that knows how to use the extension; meanwhile, even if a plugin does not support this extension, the server benefits from less network traffic from the client. Eric Blake (5): protocol: Support

...(conn->structured_replies) > + eflags |= NBD_FLAG_SEND_DF; > + > *flags = eflags; > return 0; > } > diff --git a/server/protocol.c b/server/protocol.c > index 383938f..d94cd19 100644 > --- a/server/protocol.c > +++ b/server/protocol.c > @@ -110,7 +110,7 @@ validate_request (struct connection *conn, > > /* Validate flags */ > if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE | > - NBD_CMD_FLAG_REQ_ONE)) { > + NBD_CMD_FLAG_DF | NBD_CMD_FLAG_REQ_ONE)) { > nbdkit_error ("invalid request: unknown fla...

With some crufty sed scripts we can generate functions that map from NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ"). This works on GNU sed and with FreeBSD, also with GNU sed's --posix option, so I guess the sed code is POSIX-compatible. Rich.

..._REPLY.RECV_BS_ENTRIES state. Demonstration of the assertion failure can be done with a quick patch to nbdkit (here, on top of v1.35.6): | diff --git i/server/protocol.c w/server/protocol.c | index cb530e65..6b115d99 100644 | --- i/server/protocol.c | +++ w/server/protocol.c | @@ -190,7 +190,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, | } | | /* Block status allowed? */ | - if (cmd == NBD_CMD_BLOCK_STATUS) { | + if (cmd == NBD_CMD_BLOCK_STATUS && 0) { | if (!conn->structured_replies) { | nbdkit_error ("invalid request: " |...

...request (conn); - if (!newstyle) + conn->errno_is_reliable = plugin_errno_is_reliable (conn); + if (conn->errno_is_reliable < 0) + r = -1; + else if (!newstyle) r = _negotiate_handshake_oldstyle (conn); else r = _negotiate_handshake_newstyle (conn); @@ -602,6 +605,18 @@ validate_request (struct connection *conn, return 1; /* Commands validates. */ } +/* Grab the appropriate error value. + */ +static int +get_error (struct connection *conn) +{ + int ret = 0; + + if (conn->errno_is_reliable) + ret = errno; + return ret ? ret : EIO; +} + /* This is...

...nn, 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_c...

...eflags |= NBD_FLAG_SEND_DF; >> + >> *flags = eflags; >> return 0; >> } >> diff --git a/server/protocol.c b/server/protocol.c >> index 383938f..d94cd19 100644 >> --- a/server/protocol.c >> +++ b/server/protocol.c >> @@ -110,7 +110,7 @@ validate_request (struct connection *conn, >> >> /* Validate flags */ >> if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE | >> - NBD_CMD_FLAG_REQ_ONE)) { >> + NBD_CMD_FLAG_DF | NBD_CMD_FLAG_REQ_ONE)) { >> nbdkit_error ("inval...

...BD_FLAG_SEND_FUA; + conn->can_fua = 1; + } } fl = backend->can_flush (backend, conn); if (fl == -1) return -1; if (fl) { - eflags |= NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA; + eflags |= NBD_FLAG_SEND_FLUSH; conn->can_flush = 1; } @@ -880,7 +896,7 @@ validate_request (struct connection *conn, *error = EINVAL; return false; } - if (!conn->can_flush && (flags & NBD_CMD_FLAG_FUA)) { + if (!conn->can_fua && (flags & NBD_CMD_FLAG_FUA)) { nbdkit_error ("invalid request: FUA flag not supported"); *error...

...est flags. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/connections.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/connections.c b/src/connections.c index 16c6584..44b7530 100644 --- a/src/connections.c +++ b/src/connections.c @@ -546,6 +546,13 @@ validate_request (struct connection *conn, return 0; } + /* Validate flags */ + if (flags & ~NBD_CMD_FLAG_FUA) { + nbdkit_error ("invalid request: unknown flag (0x%x)", flags); + *error = EINVAL; + return 0; + } + /* Refuse over-large read and write requests. */ if ((cmd ==...

On Thu, Aug 29, 2019 at 10:08:26PM -0500, Eric Blake wrote: > @@ -232,8 +257,8 @@ handle_request (struct connection *conn, > void *buf, struct nbdkit_extents *extents) > { > uint32_t f = 0; > - bool fua = conn->can_fua && (flags & NBD_CMD_FLAG_FUA); > int err = 0; > + int r; > > /* Clear the error, so that we know if the

...equest_send_reply (struct connection *conn) if (cmd == NBD_CMD_DISC) { debug ("client sent disconnect command, closing connection"); + pthread_mutex_unlock (&conn->read_lock); return 0; /* disconnect */ } /* Validate the request. */ if (!validate_request (conn, cmd, flags, offset, count, &error)) { if (cmd == NBD_CMD_WRITE && - skip_over_write_buffer (conn->sockin, count) < 0) + skip_over_write_buffer (conn->sockin, count) < 0) { + pthread_mutex_unlock (&conn->read_lock); return -1; +...

...+ eflags |= NBD_FLAG_SEND_FAST_ZERO; + conn->can_fast_zero = true; + } } fl = backend->can_trim (backend, conn); diff --git a/server/protocol.c b/server/protocol.c index 72f6f2a8..77a045dc 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -110,7 +110,8 @@ validate_request (struct connection *conn, /* Validate flags */ if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE | - NBD_CMD_FLAG_DF | NBD_CMD_FLAG_REQ_ONE)) { + NBD_CMD_FLAG_DF | NBD_CMD_FLAG_REQ_ONE | + NBD_CMD_FLAG_FAST_ZERO)) { nbdkit_error (&quo...

This cleans up the existing code base with regards to implicit use of errno from language bindings, then rebases the previous work in python on top of that. I'm still playing with the perl bindings, but got further after reading 'perldoc perlembed'. Eric Blake (4): plugins: Don't use bogus errno from non-C plugins plugins: Add new nbdkit_set_error() utility function python:

...rver/protocol.c index e65a078..498db1d 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -45,7 +45,7 @@ #include "internal.h" #include "byte-swapping.h" #include "minmax.h" -#include "protocol.h" +#include "nbd-protocol.h" static bool validate_request (struct connection *conn, diff --git a/tests/test-layers.c b/tests/test-layers.c index 9b595c4..1097f15 100644 --- a/tests/test-layers.c +++ b/tests/test-layers.c @@ -61,7 +61,7 @@ #include "byte-swapping.h" #include "cleanup.h" #include "exit-with-parent.h" -#inclu...