Eric Blake
2022-Feb-09 22:07 UTC
[Libguestfs] [libnbd PATCH 0/3] libnbd hardening against nbd_pread bugs
In documenting the recent CVE-2022-0485 bug in nbdcopy, I pointed out that the severity of the flaw was server-dependent (a server with structured replies caused nbdcopy to write zeroes, but a server without structured replies caused nbdcopy to leak heap contents). In fact, this series demonstrates that the severity of ignoring read errors has had server-dependent behavior in ALL stable released versins of libnbd, predating the nbdcopy bug. While the core developers were aware of that fact more than a week ago, it wasn't until this week that the Red Hat secalert team had finally decided that publicizing this fact does not constitute a second CVE fix, but is merely a data hardening technique, and therefore it is not as essential to backport to stable branches as was the nbdcopy bug fix. Other distros may disagree, so I intentionally separated this series with an eye towards easy backporting. Eric Blake (3): api: Drop server control of memset() prior to NBD_CMD_READ api: Guarantee sanitized buffer on pread failure api: Add new API nbd_set_pread_initialize() lib/internal.h | 5 +- generator/API.ml | 87 +++++++++++++++++++--- generator/C.ml | 12 ++- lib/handle.c | 17 ++++- lib/rw.c | 18 ++--- python/t/110-defaults.py | 3 +- python/t/120-set-non-defaults.py | 4 +- ocaml/tests/test_110_defaults.ml | 4 +- ocaml/tests/test_120_set_non_defaults.ml | 5 +- tests/errors.c | 34 ++++++++- golang/libnbd_110_defaults_test.go | 10 ++- golang/libnbd_120_set_non_defaults_test.go | 12 +++ 12 files changed, 179 insertions(+), 32 deletions(-) -- 2.34.1
Eric Blake
2022-Feb-09 22:07 UTC
[Libguestfs] [libnbd PATCH 1/3] api: Drop server control of memset() prior to NBD_CMD_READ
The recent CVE-2022-0485 demonstrated that clients that pass in an uninitialized buffer to nbd_pread and friends, but are then not careful about checking for read errors, were subjected to server-dependent behavior on whether their use of the buffer after failure saw sanitized zeroes or prior heap contents. Making our choice of whether to sanitize the buffer be under the control of what the server negotiates is not ideal. We commonly test with servers that support structured replies (nbdkit by default, as well as qemu-nbd), and less frequently with servers that lack them (nbd-server as of the current nbd-3.23 release, or 'nbdkit --no-sr'), thus, we are already used to the runtime speed of libnbd sanitizing a read buffer, and more likely to miss the additional security impact caused when a less-common server can turn a client bug into a heap leak. This patch makes the sanitization unconditional if we actually reach the point of issuing NBD_CMD_READ (to make it easier to backport in isolation for distros that want secure-by-default for clients that call nbd_pread correctly). Note that with just this patch, there are still some nbd_pread errors where the buffer is left uninitialized (such as calling nbd_pread before nbd_connect, or if the pread is aborted client-side due to detecting invalid parameters with nbd_set_strict_mode); but client apps are generally less likely to trip those corner cases, and any heap leak in those cases is not dependent on server behavior. Then upcoming patches will then hoist the memset() even earlier, to give guaranteed buffer contents on all error paths, then add a new API to allow clients to inform libnbd when to skip the sanitization as an optimization (either because the client sanitized the buffer itself, or has audited to ensure that an uninitialized buffer is not dereferenced). Fixes: 7c2543e9 ("lib: For structured replies, zero the buffer ahead of time.", v0.1) --- lib/rw.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/rw.c b/lib/rw.c index 4ade7508..b5d3dc44 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2020, 2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -250,9 +250,11 @@ nbd_internal_command_common (struct nbd_handle *h, * ahead of time which avoids any security problems. I measured the * overhead of this and for non-TLS there is no measurable overhead * in the highly intensive loopback case. For TLS we get a - * performance gain, go figure. + * performance gain, go figure. For an older server with only + * simple replies, it's still better to do the same memset() so we + * don't have behavior that is server-dependent. */ - if (h->structured_replies && cmd->data && type == NBD_CMD_READ) + if (cmd->data && type == NBD_CMD_READ) memset (cmd->data, 0, cmd->count); /* Add the command to the end of the queue. Kick the state machine -- 2.34.1
Eric Blake
2022-Feb-09 22:07 UTC
[Libguestfs] [libnbd PATCH 2/3] api: Guarantee sanitized buffer on pread failure
As mentioned in the previous patch, we left the state of the buffer undefined if we fail pread prior to attempting NBD_CMD_READ. Better is to tweak the generator to sanitize the buffer unconditionally, as a way of hardening against potential bugs in client applications that fail to check for errors, but which should not be leaking uninitialized data. In the process, we can document that the contents of buf are now merely unspecified, rather than undefined (valgrind will no longer complain if you read buf, regardless of how nbd_pread failed). As always calling memset() can be a performance hit if the client also sanitizes the buffer or is willing to take the audit risk, the next patch will add a knob that allows the client to control when this happens. --- generator/API.ml | 24 ++++++++++++++---------- generator/C.ml | 11 ++++++++++- lib/rw.c | 18 +++++++----------- tests/errors.c | 9 ++++++++- 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/generator/API.ml b/generator/API.ml index 012016bc..98f90031 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -1831,7 +1831,8 @@ "pread", { The C<flags> parameter must be C<0> for now (it exists for future NBD protocol extensions). -Note that if this command fails, the contents of C<buf> are undefined." +Note that if this command fails, it is unspecified whether the contents +of C<buf> will read as zero or as partial results from the server." ^ strict_call_description; see_also = [Link "aio_pread"; Link "pread_structured"; Link "get_block_size"; Link "set_strict_mode"]; @@ -1918,7 +1919,8 @@ "pread_structured", { this, see L<nbd_can_df(3)>). Libnbd does not validate that the server actually obeys the flag. -Note that if this command fails, the contents of C<buf> are undefined." +Note that if this command fails, it is unspecified whether the contents +of C<buf> will read as zero or as partial results from the server." ^ strict_call_description; see_also = [Link "can_df"; Link "pread"; Link "aio_pread_structured"; Link "get_block_size"; @@ -2459,10 +2461,11 @@ "aio_pread", { as described in L<libnbd(3)/Completion callbacks>. Note that you must ensure C<buf> is valid until the command has -completed. Furthermore, the contents of C<buf> are undefined if the -C<error> parameter to C<completion_callback> is set, or if -L<nbd_aio_command_completed(3)> reports failure. Other parameters behave -as documented in L<nbd_pread(3)>." +completed. Furthermore, if the C<error> parameter to +C<completion_callback> is set or if L<nbd_aio_command_completed(3)> +reports failure, it is unspecified whether the contents +of C<buf> will read as zero or as partial results from the server. +Other parameters behave as documented in L<nbd_pread(3)>." ^ strict_call_description; example = Some "examples/aio-connect-read.c"; see_also = [SectionLink "Issuing asynchronous commands"; @@ -2487,10 +2490,11 @@ "aio_pread_structured", { as described in L<libnbd(3)/Completion callbacks>. Note that you must ensure C<buf> is valid until the command has -completed. Furthermore, the contents of C<buf> are undefined if the -C<error> parameter to C<completion_callback> is set, or if -L<nbd_aio_command_completed(3)> reports failure. Other parameters behave -as documented in L<nbd_pread_structured(3)>." +completed. Furthermore, if the C<error> parameter to +C<completion_callback> is set or if L<nbd_aio_command_completed(3)> +reports failure, it is unspecified whether the contents +of C<buf> will read as zero or as partial results from the server. +Other parameters behave as documented in L<nbd_pread_structured(3)>." ^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; Link "aio_pread"; Link "pread_structured"; diff --git a/generator/C.ml b/generator/C.ml index 797af531..4a5bb589 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -1,6 +1,6 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* nbd client library in userspace: generate the C API and documentation - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -491,6 +491,15 @@ let pr "\n" ); + (* Sanitize read buffers before any error is possible. *) + List.iter ( + function + | BytesOut (n, count) + | BytesPersistOut (n, count) -> + pr " memset (%s, 0, %s);\n" n count + | _ -> () + ) args; + (* Check current state is permitted for this call. *) if permitted_states <> [] then ( let value = match errcode with diff --git a/lib/rw.c b/lib/rw.c index b5d3dc44..1202d71c 100644 --- a/lib/rw.c +++ b/lib/rw.c @@ -244,18 +244,14 @@ nbd_internal_command_common (struct nbd_handle *h, if (cb) cmd->cb = *cb; - /* If structured replies were negotiated then we trust the server to - * send back sufficient data to cover the whole buffer. It's tricky - * to check this, so an easier thing is simply to zero the buffer - * ahead of time which avoids any security problems. I measured the - * overhead of this and for non-TLS there is no measurable overhead - * in the highly intensive loopback case. For TLS we get a - * performance gain, go figure. For an older server with only - * simple replies, it's still better to do the same memset() so we - * don't have behavior that is server-dependent. + /* For NBD_CMD_READ, cmd->data was pre-zeroed in the prologue + * created by the generator. Thus, if a (non-compliant) server with + * structured replies fails to send back sufficient data to cover + * the whole buffer, we still behave as if it had sent zeroes for + * those portions, rather than leaking any uninitialized data, and + * without having to complicate our state machine to track which + * portions of the read buffer were actually populated. */ - if (cmd->data && type == NBD_CMD_READ) - memset (cmd->data, 0, cmd->count); /* Add the command to the end of the queue. Kick the state machine * if there is no other command being processed, otherwise, it will diff --git a/tests/errors.c b/tests/errors.c index 2c800c7c..f597b7ee 100644 --- a/tests/errors.c +++ b/tests/errors.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2021 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -214,6 +214,7 @@ main (int argc, char *argv[]) /* Issue a connected command when not connected. */ + buf[0] = '1'; if (nbd_pread (nbd, buf, 512, 0, 0) != -1) { fprintf (stderr, "%s: test failed: " "nbd_pread did not fail on non-connected handle\n", @@ -221,6 +222,12 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } check (ENOTCONN, "nbd_pread: "); + if (buf[0] != '\0') { + fprintf (stderr, "%s: test failed: " + "nbd_pread did not sanitize buffer on error\n", + argv[0]); + exit (EXIT_FAILURE); + } /* Request a name that is too long. */ memset (buf, 'a', 4999); -- 2.34.1
Eric Blake
2022-Feb-09 22:07 UTC
[Libguestfs] [libnbd PATCH 3/3] api: Add new API nbd_set_pread_initialize()
The recent patch series for CVE-2022-0485 demonstrated that when applications using libnbd are not careful about error checking, the difference on whether a data leak is at least sanitized (all zeroes, partial reads, or data leftover from a prior read) vs. a dangerous information leak (uninitialized data from the heap) was partly under libnbd's control. The previous two patches changed libnbd to always sanitize, as a security hardening technique that prevents heap leaks no matter how buggy the client app is. But a blind memset() also adds an execution delay, even if it doesn't show up as the hot spot in our profiling to the slower time spent with network traffic. At any rate, if client apps choose to pre-initialize their buffers, or otherwise audit their code to take on their own risk about not dereferencing a buffer on failure paths, then the time spent by libnbd doing memset() is wasted; so it is worth adding a knob to let a user opt in to faster execution at the expense of giving up our memset() hardening on their behalf. --- lib/internal.h | 5 +- generator/API.ml | 79 +++++++++++++++++++--- generator/C.ml | 3 +- lib/handle.c | 17 ++++- python/t/110-defaults.py | 3 +- python/t/120-set-non-defaults.py | 4 +- ocaml/tests/test_110_defaults.ml | 4 +- ocaml/tests/test_120_set_non_defaults.ml | 5 +- tests/errors.c | 25 ++++++- golang/libnbd_110_defaults_test.go | 10 ++- golang/libnbd_120_set_non_defaults_test.go | 12 ++++ 11 files changed, 148 insertions(+), 19 deletions(-) diff --git a/lib/internal.h b/lib/internal.h index 0e205aba..525499a9 100644 --- a/lib/internal.h +++ b/lib/internal.h @@ -1,5 +1,5 @@ /* nbd client library in userspace: internal definitions - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -123,6 +123,9 @@ struct nbd_handle { /* Full info mode. */ bool full_info; + /* Sanitization for pread. */ + bool pread_initialize; + /* Global flags from the server. */ uint16_t gflags; diff --git a/generator/API.ml b/generator/API.ml index 98f90031..9b7eb545 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -778,6 +778,44 @@ "get_handshake_flags", { Link "aio_is_created"; Link "aio_is_ready"]; }; + "set_pread_initialize", { + default_call with + args = [Bool "request"]; ret = RErr; + shortdesc = "control whether libnbd pre-initializes read buffers"; + longdesc = "\ +By default, libnbd will pre-initialize the contents of a buffer +passed to calls such as L<nbd_pread(3)> to all zeroes prior to checking +for any other errors, so that even if a client application passed in an +uninitialized buffer but fails to check for errors, it will not result +in a potential security risk caused by an accidental leak of prior heap +contents. However, for a client application that has audited that an +uninitialized buffer is never dereferenced, or which performs its own +pre-initialization, libnbd's sanitization efforts merely pessimize +performance. + +Calling this function with C<request> set to false tells libnbd to +skip the buffer initialization step in read commands."; + see_also = [Link "get_pread_initialize"; + Link "set_strict_mode"; + Link "pread"; Link "pread_structured"; Link "aio_pread"; + Link "aio_pread_structured"]; + }; + + "get_pread_initialize", { + default_call with + args = []; ret = RBool; + may_set_error = false; + shortdesc = "see whether libnbd pre-initializes read buffers"; + longdesc = "\ +Return whether libnbd performs a pre-initialization of a buffer passed +to L<nbd_pread(3)> and similar to all zeroes, as set by +L<nbd_set_pread_initialize(3)>."; + see_also = [Link "set_pread_initialize"; + Link "set_strict_mode"; + Link "pread"; Link "pread_structured"; Link "aio_pread"; + Link "aio_pread_structured"]; + }; + "set_strict_mode", { default_call with args = [ Flags ("flags", strict_flags) ]; ret = RErr; @@ -1831,11 +1869,16 @@ "pread", { The C<flags> parameter must be C<0> for now (it exists for future NBD protocol extensions). -Note that if this command fails, it is unspecified whether the contents -of C<buf> will read as zero or as partial results from the server." +Note that if this command fails, and L<nbd_get_pread_initialize(3)> +returns true, then libnbd sanitized C<buf>, but it is unspecified +whether the contents of C<buf> will read as zero or as partial results +from the server. If L<nbd_get_pread_initialize(3)> returns false, +then libnbd did not sanitize C<buf>, and the contents are undefined +on failure." ^ strict_call_description; see_also = [Link "aio_pread"; Link "pread_structured"; - Link "get_block_size"; Link "set_strict_mode"]; + Link "get_block_size"; Link "set_strict_mode"; + Link "set_pread_initialize"]; example = Some "examples/fetch-first-sector.c"; }; @@ -1919,8 +1962,12 @@ "pread_structured", { this, see L<nbd_can_df(3)>). Libnbd does not validate that the server actually obeys the flag. -Note that if this command fails, it is unspecified whether the contents -of C<buf> will read as zero or as partial results from the server." +Note that if this command fails, and L<nbd_get_pread_initialize(3)> +returns true, then libnbd sanitized C<buf>, but it is unspecified +whether the contents of C<buf> will read as zero or as partial results +from the server. If L<nbd_get_pread_initialize(3)> returns false, +then libnbd did not sanitize C<buf>, and the contents are undefined +on failure." ^ strict_call_description; see_also = [Link "can_df"; Link "pread"; Link "aio_pread_structured"; Link "get_block_size"; @@ -2463,8 +2510,13 @@ "aio_pread", { Note that you must ensure C<buf> is valid until the command has completed. Furthermore, if the C<error> parameter to C<completion_callback> is set or if L<nbd_aio_command_completed(3)> -reports failure, it is unspecified whether the contents -of C<buf> will read as zero or as partial results from the server. +reports failure, and if L<nbd_get_pread_initialize(3)> returns true, +then libnbd sanitized C<buf>, but it is unspecified whether the +contents of C<buf> will read as zero or as partial results from the +server. If L<nbd_get_pread_initialize(3)> returns false, then +libnbd did not sanitize C<buf>, and the contents are undefined +on failure. + Other parameters behave as documented in L<nbd_pread(3)>." ^ strict_call_description; example = Some "examples/aio-connect-read.c"; @@ -2492,8 +2544,13 @@ "aio_pread_structured", { Note that you must ensure C<buf> is valid until the command has completed. Furthermore, if the C<error> parameter to C<completion_callback> is set or if L<nbd_aio_command_completed(3)> -reports failure, it is unspecified whether the contents -of C<buf> will read as zero or as partial results from the server. +reports failure, and if L<nbd_get_pread_initialize(3)> returns true, +then libnbd sanitized C<buf>, but it is unspecified whether the +contents of C<buf> will read as zero or as partial results from the +server. If L<nbd_get_pread_initialize(3)> returns false, then +libnbd did not sanitize C<buf>, and the contents are undefined +on failure. + Other parameters behave as documented in L<nbd_pread_structured(3)>." ^ strict_call_description; see_also = [SectionLink "Issuing asynchronous commands"; @@ -3136,6 +3193,10 @@ let first_version "get_private_data", (1, 8); "get_uri", (1, 8); + (* Added in 1.11.x development cycle, will be stable and supported in 1.12. *) + "set_pread_initialize", (1, 12); + "get_pread_initialize", (1, 12); + (* These calls are proposed for a future version of libnbd, but * have not been added to any released version so far. "get_tls_certificates", (1, ??); diff --git a/generator/C.ml b/generator/C.ml index 4a5bb589..2b6198c3 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -496,7 +496,8 @@ let function | BytesOut (n, count) | BytesPersistOut (n, count) -> - pr " memset (%s, 0, %s);\n" n count + pr " if (h->pread_initialize)\n"; + pr " memset (%s, 0, %s);\n" n count | _ -> () ) args; diff --git a/lib/handle.c b/lib/handle.c index cbb37e89..4f00c059 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -1,5 +1,5 @@ /* NBD client library in userspace - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -64,6 +64,7 @@ nbd_create (void) h->unique = 1; h->tls_verify_peer = true; h->request_sr = true; + h->pread_initialize = true; h->uri_allow_transports = LIBNBD_ALLOW_TRANSPORT_MASK; h->uri_allow_tls = LIBNBD_TLS_ALLOW; @@ -393,6 +394,20 @@ nbd_unlocked_get_handshake_flags (struct nbd_handle *h) return h->gflags; } +int +nbd_unlocked_set_pread_initialize (struct nbd_handle *h, bool request) +{ + h->pread_initialize = request; + return 0; +} + +/* NB: may_set_error = false. */ +int +nbd_unlocked_get_pread_initialize (struct nbd_handle *h) +{ + return h->pread_initialize; +} + int nbd_unlocked_set_strict_mode (struct nbd_handle *h, uint32_t flags) { diff --git a/python/t/110-defaults.py b/python/t/110-defaults.py index fb961cfd..a4262dae 100644 --- a/python/t/110-defaults.py +++ b/python/t/110-defaults.py @@ -1,5 +1,5 @@ # libnbd Python bindings -# Copyright (C) 2010-2020 Red Hat Inc. +# Copyright (C) 2010-2022 Red Hat Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -22,5 +22,6 @@ assert h.get_export_name() == "" assert h.get_full_info() is False assert h.get_tls() == nbd.TLS_DISABLE assert h.get_request_structured_replies() is True +assert h.get_pread_initialize() is True assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK assert h.get_opt_mode() is False diff --git a/python/t/120-set-non-defaults.py b/python/t/120-set-non-defaults.py index 3da0c23e..e71c6ad0 100644 --- a/python/t/120-set-non-defaults.py +++ b/python/t/120-set-non-defaults.py @@ -1,5 +1,5 @@ # libnbd Python bindings -# Copyright (C) 2010-2020 Red Hat Inc. +# Copyright (C) 2010-2022 Red Hat Inc. # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -33,6 +33,8 @@ if h.supports_tls(): assert h.get_tls() == nbd.TLS_ALLOW h.set_request_structured_replies(False) assert h.get_request_structured_replies() is False +h.set_pread_initialize(False) +assert h.get_pread_initialize() is False try: h.set_handshake_flags(nbd.HANDSHAKE_FLAG_MASK + 1) assert False diff --git a/ocaml/tests/test_110_defaults.ml b/ocaml/tests/test_110_defaults.ml index b36949f0..04aa744a 100644 --- a/ocaml/tests/test_110_defaults.ml +++ b/ocaml/tests/test_110_defaults.ml @@ -1,6 +1,6 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* libnbd OCaml test case - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -28,6 +28,8 @@ let assert (tls = NBD.TLS.DISABLE); let sr = NBD.get_request_structured_replies nbd in assert (sr = true); + let init = NBD.get_pread_initialize nbd in + assert (init = true); let flags = NBD.get_handshake_flags nbd in assert (flags = NBD.HANDSHAKE_FLAG.mask); let opt = NBD.get_opt_mode nbd in diff --git a/ocaml/tests/test_120_set_non_defaults.ml b/ocaml/tests/test_120_set_non_defaults.ml index 67928bb5..f9498076 100644 --- a/ocaml/tests/test_120_set_non_defaults.ml +++ b/ocaml/tests/test_120_set_non_defaults.ml @@ -1,6 +1,6 @@ (* hey emacs, this is OCaml code: -*- tuareg -*- *) (* libnbd OCaml test case - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -42,6 +42,9 @@ let NBD.set_request_structured_replies nbd false; let sr = NBD.get_request_structured_replies nbd in assert (sr = false); + NBD.set_pread_initialize nbd false; + let init = NBD.get_pread_initialize nbd in + assert (init = false); (try NBD.set_handshake_flags nbd [ NBD.HANDSHAKE_FLAG.UNKNOWN 2 ]; assert false diff --git a/tests/errors.c b/tests/errors.c index f597b7ee..0298da88 100644 --- a/tests/errors.c +++ b/tests/errors.c @@ -213,7 +213,15 @@ main (int argc, char *argv[]) } - /* Issue a connected command when not connected. */ + /* Issue a connected command when not connected. pread_initialize defaults + * to set. + */ + if (nbd_get_pread_initialize (nbd) != 1) { + fprintf (stderr, "%s: test failed: " + "nbd_get_pread_initialize gave unexpected result\n", + argv[0]); + exit (EXIT_FAILURE); + } buf[0] = '1'; if (nbd_pread (nbd, buf, 512, 0, 0) != -1) { fprintf (stderr, "%s: test failed: " @@ -294,7 +302,14 @@ main (int argc, char *argv[]) } check (EINVAL, "nbd_aio_command_completed: "); - /* Read from an invalid offset, client-side */ + /* Read from an invalid offset, client-side. When pread_initialize is off, + * libnbd should not have touched our buffer. + */ + if (nbd_set_pread_initialize (nbd, false) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + buf[0] = '1'; strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_BOUNDS; if (nbd_set_strict_mode (nbd, strict) == -1) { fprintf (stderr, "%s\n", nbd_get_error ()); @@ -307,6 +322,12 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } check (EINVAL, "nbd_aio_pread: "); + if (buf[0] != '1') { + fprintf (stderr, "%s: test failed: " + "nbd_pread incorrectly sanitized buffer on client-side error\n", + argv[0]); + exit (EXIT_FAILURE); + } /* We guarantee callbacks will be freed even on all error paths. */ if (nbd_aio_pread_structured (nbd, buf, 512, -1, diff --git a/golang/libnbd_110_defaults_test.go b/golang/libnbd_110_defaults_test.go index b3ceb45d..ca7c1c41 100644 --- a/golang/libnbd_110_defaults_test.go +++ b/golang/libnbd_110_defaults_test.go @@ -1,5 +1,5 @@ /* libnbd golang tests - * Copyright (C) 2013-2021 Red Hat Inc. + * Copyright (C) 2013-2022 Red Hat Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -59,6 +59,14 @@ func Test110Defaults(t *testing.T) { t.Fatalf("unexpected structured replies state") } + init, err := h.GetPreadInitialize() + if err != nil { + t.Fatalf("could not get pread initialize state: %s", err) + } + if init != true { + t.Fatalf("unexpected pread initialize state") + } + flags, err := h.GetHandshakeFlags() if err != nil { t.Fatalf("could not get handshake flags: %s", err) diff --git a/golang/libnbd_120_set_non_defaults_test.go b/golang/libnbd_120_set_non_defaults_test.go index f112456c..029f0db3 100644 --- a/golang/libnbd_120_set_non_defaults_test.go +++ b/golang/libnbd_120_set_non_defaults_test.go @@ -93,6 +93,18 @@ func Test120SetNonDefaults(t *testing.T) { t.Fatalf("unexpected structured replies state") } + err = h.SetPreadInitialize(false) + if err != nil { + t.Fatalf("could not set pread initialize state: %s", err) + } + init, err := h.GetPreadInitialize() + if err != nil { + t.Fatalf("could not get pread initialize state: %s", err) + } + if init != false { + t.Fatalf("unexpected pread initialize state") + } + err = h.SetHandshakeFlags(HANDSHAKE_FLAG_MASK + 1) if err == nil { t.Fatalf("expect failure for out-of-range flags") -- 2.34.1