search for: skip_over_write_buffer

When facing a malicious client that is sending bogus NBD_CMD_READ, we should make sure that we never end up in a situation where we could try to treat the tail from a command that we diagnosed as bad as being further commands. Eric Blake (2): connections: Report mid-message EOF as fatal connections: Hang up early on insanely large WRITE requests src/connections.c | 35

...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; + } + pthread_mutex_unlock (&conn->read_lock); goto send_reply; } @@ -931,8 +945,11 @@ recv_request_send...

...+ 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; > + } > + pthread_mutex_unlock (&conn->read_lock); > goto send_reply; > } >...

I want to make my nbd forwarding plugin fully parallel - but to do that, I first need to make nbdkit itself fully parallel ;) With this series, I was finally able to demonstrate out-of-order responses when using qemu-io (which is great at sending back-to-back requests prior to waiting for responses) coupled with the nbd file plugin (which has a great feature of rdelay and wdelay, to make it

...threadlocal_buffer ((size_t) count); > if (buf == NULL) { > - out_of_memory: > - perror ("malloc"); > error = ENOMEM; Old code called perror() when nbdkit_extents_new() failed... > if (cmd == NBD_CMD_WRITE && > skip_over_write_buffer (conn->sockin, count) < 0) > @@ -673,8 +673,10 @@ protocol_recv_request_send_reply (struct connection *conn) > /* Allocate the extents list for block status only. */ > if (cmd == NBD_CMD_BLOCK_STATUS) { > extents = nbdkit_extents_new (offset, conn->exportsize);...

...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

This bug was found by Eric Blake. In the .pread method we allocate a buffer in the server and pass it to the plugin. The plugin is supposed to fill it with data. The buffer was uninitialized so initially contained random heap data, but that's OK provided the plugin fully overwrote it with data. All correctly written plugins ought to do this, however there is the possibility of an

...to just silently disconnect. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/connections.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/connections.c b/src/connections.c index d0ef6a5..8dc1925 100644 --- a/src/connections.c +++ b/src/connections.c @@ -879,6 +879,11 @@ skip_over_write_buffer (int sock, size_t count) char buf[BUFSIZ]; ssize_t r; + if (count > MAX_REQUEST_SIZE * 2) { + nbdkit_error ("write request too large to skip"); + return -1; + } + while (count > 0) { r = read (sock, buf, count > BUFSIZ ? BUFSIZ : count); if (r == -1) {...

.... Signed-off-by: Eric Blake <eblake@redhat.com> --- server/protocol.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/server/protocol.c b/server/protocol.c index a52bb56..0a9f73c 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -326,7 +326,7 @@ skip_over_write_buffer (int sock, size_t count) /* Convert a system errno to an NBD_E* error code. */ static int -nbd_errno (int error) +nbd_errno (int error, bool flag_df) { switch (error) { case 0: @@ -349,7 +349,9 @@ nbd_errno (int error) return NBD_ESHUTDOWN; #endif case EOVERFLOW: - return NBD_...

...READ || cmd == NBD_CMD_WRITE) { - buf = malloc (count); + buf = calloc (1, count); if (buf == NULL) { out_of_memory: - perror ("malloc"); + perror ("calloc"); error = ENOMEM; if (cmd == NBD_CMD_WRITE && skip_over_write_buffer (conn->sockin, count) < 0) -- 2.20.1

I've posted some of these patches or ideas before; but now I'm confident enough with the series that it should be ready to push; at any rate, I can now run test-socket-activation in a tight loop without triggering any crashes or hangs. With this in place, I'm going back to work on making the nbd forwarder wort with the parallel thread model. Eric Blake (8): sockets: Use

Version 1 was here: https://www.redhat.com/archives/libguestfs/2019-April/msg00144.html Version 2 makes a couple of much larger changes: The OCaml patch changes the API of the pread method so it matches what other language bindings are already doing, ie. get the language plugin to return a newly allocated buffer, check it is long enough, copy out the data. The server patch implements a

...if (cmd == NBD_CMD_READ || cmd == NBD_CMD_WRITE) { - buf = malloc (count); + buf = threadlocal_buffer ((size_t) count); if (buf == NULL) { - out_of_memory: - perror ("malloc"); error = ENOMEM; if (cmd == NBD_CMD_WRITE && skip_over_write_buffer (conn->sockin, count) < 0) @@ -673,8 +673,10 @@ protocol_recv_request_send_reply (struct connection *conn) /* Allocate the extents list for block status only. */ if (cmd == NBD_CMD_BLOCK_STATUS) { extents = nbdkit_extents_new (offset, conn->exportsize); - if (extents...

...: - if (!(flags & NBD_CMD_FLAG_NO_HOLE)) - f |= NBDKIT_FLAG_MAY_TRIM; - if (fua) - f |= NBDKIT_FLAG_FUA; - if (backend->zero (backend, conn, count, offset, f, &err) == -1) - return err; - break; - - default: - abort (); - } - - return 0; -} - -static int -skip_over_write_buffer (int sock, size_t count) -{ - char buf[BUFSIZ]; - ssize_t r; - - if (count > MAX_REQUEST_SIZE * 2) { - nbdkit_error ("write request too large to skip"); - return -1; - } - - while (count > 0) { - r = read (sock, buf, count > BUFSIZ ? BUFSIZ : count); - if (r == -...

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

...ur fio patches that might help us answer that question). > if (buf == NULL) { > out_of_memory: > - perror ("malloc"); > + perror ("calloc"); > error = ENOMEM; > if (cmd == NBD_CMD_WRITE && > skip_over_write_buffer (conn->sockin, count) < 0) > Is there any smarter algorithm we could use? Right now, we are doing a malloc()/free() per READ/WRITE. And although libc malloc() should in general ty to be efficient on frequent malloc reuse, we can often still be faster by reusing allocations rather than do...

The concern is a client is blocked while processing a request. The nbdkit server design requires a thread per request being processed regardless of the number of connections or clients. We want to run 1000's of requests in parallel without needing a thread at nbdkit layer per request in flight. Our plugin layer is built around boost asio and a few threads in a worker pool running an io

...t.com> > --- > server/protocol.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/server/protocol.c b/server/protocol.c > index a52bb56..0a9f73c 100644 > --- a/server/protocol.c > +++ b/server/protocol.c > @@ -326,7 +326,7 @@ skip_over_write_buffer (int sock, size_t count) > > /* Convert a system errno to an NBD_E* error code. */ > static int > -nbd_errno (int error) > +nbd_errno (int error, bool flag_df) > { > switch (error) { > case 0: > @@ -349,7 +349,9 @@ nbd_errno (int error) > return NBD_ESHU...

...ocks; + data.nr_blocks = *nr_blocks; + if (nbdkit_extents_foreach (extents_map, + copy_extents, &data, + foreach_flags, + offset, (uint64_t) count) == -1) + return errno; + + return 0; +} + static int skip_over_write_buffer (int sock, size_t count) { @@ -359,6 +490,60 @@ send_structured_reply_read (struct connection *conn, return 1; /* command processed ok */ } +static int +send_structured_reply_block_status (struct connection *conn, + uint64_t handle, +...

..., offset, f, &err) == -1) { - assert (err && err != ENOTSUP && err != EOPNOTSUPP); + assert (err); + if (!(flags & NBD_CMD_FLAG_FAST_ZERO)) + assert (err != ENOTSUP && err != EOPNOTSUPP); return err; } break; @@ -368,7 +388,7 @@ skip_over_write_buffer (int sock, size_t count) /* Convert a system errno to an NBD_E* error code. */ static int -nbd_errno (int error, bool flag_df) +nbd_errno (int error, uint16_t flags) { switch (error) { case 0: @@ -390,10 +410,17 @@ nbd_errno (int error, bool flag_df) case ESHUTDOWN: return NBD_ESH...