search for: protocol_recv_request_send_repli

This commit implements the core NBD protocol for the "base:allocation" Block Status replies. --- server/internal.h | 7 + server/protocol.h | 17 +- server/protocol-handshake-newstyle.c | 79 ++++++++- server/protocol.c | 248 +++++++++++++++++++++++++-- 4 files changed, 335 insertions(+), 16 deletions(-) diff --git

This commit implements the core NBD protocol for the "base:allocation" Block Status replies. --- server/internal.h | 7 ++ server/protocol.h | 15 +++ server/protocol-handshake-newstyle.c | 81 ++++++++++++++- server/protocol.c | 141 ++++++++++++++++++++++++--- 4 files changed, 229 insertions(+), 15 deletions(-) diff --git

GCC 9.0.1 isn't smart enough to work out that buf is initialized on all the relevant paths that lead to backend->pwrite being called. By initializing it we can avoid the warning. protocol.c: In function ‘protocol_recv_request_send_reply’: protocol.c:241:9: error: ‘buf’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 241 | if (backend->pwrite (backend,

If the plugin .pread method did not fill the buffer with data then the contents of the heap could be leaked back to the client. To avoid this create a thread-local data buffer which is initialized to zero and expanded (with zeroes) as required. This buffer is shared between pread and pwrite which makes the code a little bit simpler. Also this may improve locality by reusing the same memory for

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

On 4/23/19 10:09 AM, Richard W.M. Jones wrote: > If the plugin .pread method did not fill the buffer with data then the > contents of the heap could be leaked back to the client. To avoid > this create a thread-local data buffer which is initialized to zero > and expanded (with zeroes) as required. > > This buffer is shared between pread and pwrite which makes the code a >

Patch 2 certainly allows you to set the thread model. However patch 3 shows that if you set it to nbdkit.THREAD_MODEL_PARALLEL it will crash. If you look closely at the stack trace (attached below) you can see that ignoring threads which are in parts of nbdkit unrelated to Python: Thread 4: In pread, waiting in time.sleep(). This thread has released the GIL. Thread 2: Started to

Since commit 86fdb48c6a5362d66865493d9d2172166f99722e we have stored the connection object in thread-local storage. In this very large, but mostly mechanical change we stop passing the connection pointer around everywhere, and instead use the value stored in thread-local storage. This assumes a 1-1 mapping between the connection and the current thread which is true in *most* places.

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.

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

Previously, we were squashing EOVERFLOW into EINVAL; continue to do so at points in the protocol where the client may not be expecting EOVERFLOW. 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

If the plugin .pread method did not fill the buffer with data then the contents of the heap could be leaked back to the client. The simplest way to avoid any possibility of this happening is to zero the buffer before passing it to the .pread method. I have checked plugins shipped with nbdkit and none of them are vulnerable in this way. They all do one of these things: - They fully set the

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > This commit implements the core NBD protocol for the "base:allocation" > Block Status replies. > --- > server/internal.h | 7 ++ > server/protocol.h | 15 +++ > server/protocol-handshake-newstyle.c | 81 ++++++++++++++- > server/protocol.c | 141

Not sure what version we're up to, but this reimplements extents using the new simpler structure described in this thread: https://www.redhat.com/archives/libguestfs/2019-March/msg00077.html I also fixed most of the things that Eric pointed out in the previous review, although I need to go back over his replies and check I've got everything. This needs a bit more testing. However the

I want to post this but mainly for discussion and early review. It's not safe for these patches to all go upstream yet (because not all filters have been checked/adjusted), but if any patches were to go upstream then probably 1 & 2 only are safe. File, VDDK, memory and data plugins all work, although I have only done minimal testing on them. The current tests, such as they are, all

We don't know how long a plugin's .get_size() will take, but we also documented that it shouldn't change per connection and therefore can be cached. It's not hard to see that we have to consult the size per connection (see commit b3a43ccd for a test that purposefully exposes different sizes to separate clients), nor to search the code to see we already cache it at the protocol

I'm not sure exactly which version we're up to, but let's say it's version 4. I'm a lot happier with this version: - all filters have been reviewed and changed where I think that's necessary - can_extents is properly defined and implemented now - NBD protocol is followed - I believe it addresses all previous review points where possible The "only" thing

On 4/23/19 4:01 AM, Richard W.M. Jones wrote: > If the plugin .pread method did not fill the buffer with data then the > contents of the heap could be leaked back to the client. The simplest > way to avoid any possibility of this happening is to zero the buffer > before passing it to the .pread method. > > I have checked plugins shipped with nbdkit and none of them are >

On Mon, Apr 22, 2019 at 07:50:22PM -0500, Eric Blake wrote: > Previously, we were squashing EOVERFLOW into EINVAL; continue to do so > at points in the protocol where the client may not be expecting > EOVERFLOW. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > server/protocol.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) >