[PATCH libnbd] api: Allow NBD URIs to be restricted.
by Richard W.M. Jones
Previous discussion:
https://www.redhat.com/archives/libguestfs/2019-August/msg00102.html
Last night I experimentally added support for URIs that contain the
query parameter tls-psk-file, as part of rewriting the tests to cover
more of the URI code. So you can now have a URI like:
nbds://alice@localhost/?tls-psk-file=keys.psk
However there's an obvious security problem here because now any
libnbd program which takes URIs from less trusted sources will open a
local file under the user's control.
So it's time to restrict what can appear in URIs.
I've added three new APIs for this purpose, see generator/generator in
this patch for documentation. The defaults are fairly liberal, except
we do prevent opening local files (except socket) by default.
Rich.
4 years, 6 months
[PATCH libnbd] lib: Use GCC hints to move debug and error handling code out of hot paths.
by Richard W.M. Jones
---
generator/generator | 6 +++---
lib/crypto.c | 2 +-
lib/internal.h | 14 +++++++++++++-
3 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/generator/generator b/generator/generator
index c2ff0db..eb3408d 100755
--- a/generator/generator
+++ b/generator/generator
@@ -4512,7 +4512,7 @@ let generate_lib_api_c () =
let value = match errcode with
| Some value -> value
| None -> assert false in
- pr " if (!%s_in_permitted_state (h)) {\n" name;
+ pr " if (unlikely (!%s_in_permitted_state (h))) {\n" name;
pr " ret = %s;\n" value;
pr " goto out;\n";
pr " }\n";
@@ -4525,7 +4525,7 @@ let generate_lib_api_c () =
| Some value -> value
| None -> assert false in
let mask = List.fold_left (lor) 0 (List.map snd flags) in
- pr " if ((%s & ~%d) != 0) {\n" n mask;
+ pr " if (unlikely ((%s & ~%d) != 0)) {\n" n mask;
pr " set_error (EINVAL, \"%%s: invalid value for flag: %%d\",\n";
pr " \"%s\", %s);\n" n n;
pr " ret = %s;\n" value;
@@ -4658,7 +4658,7 @@ let generate_lib_api_c () =
pr ");\n"
(* Print the trace when we leave a call with debugging enabled. *)
and print_trace_leave ret =
- pr " if (h->debug) {\n";
+ pr " if_debug (h) {\n";
let errcode = errcode_of_ret ret in
(match errcode with
| Some r ->
diff --git a/lib/crypto.c b/lib/crypto.c
index 07d06c0..8d86911 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -676,7 +676,7 @@ nbd_internal_crypto_handshake (struct nbd_handle *h)
void
nbd_internal_crypto_debug_tls_enabled (struct nbd_handle *h)
{
- if (h->debug) {
+ if_debug (h) {
const gnutls_session_t session = h->sock->u.tls.session;
const gnutls_cipher_algorithm_t cipher = gnutls_cipher_get (session);
const gnutls_kx_algorithm_t kx = gnutls_kx_get (session);
diff --git a/lib/internal.h b/lib/internal.h
index 50c0a9b..894e437 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -40,6 +40,18 @@
#include "states.h"
#include "unlocked.h"
+/* Define unlikely macro, but only for GCC. These are used to move
+ * debug and error handling code out of hot paths, making the hot path
+ * into common functions use less instruction cache.
+ */
+#if defined(__GNUC__)
+#define unlikely(x) __builtin_expect (!!(x), 0)
+#define if_debug(h) if (unlikely ((h)->debug))
+#else
+#define unlikely(x) (x)
+#define if_debug(h) if ((h)->debug)
+#endif
+
/* MSG_MORE is an optimization. If not present, ignore it. */
#ifndef MSG_MORE
#define MSG_MORE 0
@@ -329,7 +341,7 @@ extern void nbd_internal_crypto_debug_tls_enabled (struct nbd_handle *);
extern void nbd_internal_debug (struct nbd_handle *h, const char *fs, ...);
#define debug(h, fs, ...) \
do { \
- if ((h)->debug) \
+ if_debug ((h)) \
nbd_internal_debug ((h), (fs), ##__VA_ARGS__); \
} while (0)
--
2.23.0
4 years, 6 months
[PATCH nbdkit 0/3] server: Fix crash on close.
by Richard W.M. Jones
This fixes the long-standing crash on close when nbdkit exits.
I did try first to fix threads so we're using a proper thread pool,
but that's difficult to implement. So this does the minimal change
needed to fix the crash instead.
There are still two segfaults that happen during running the test
suite. One is deliberately caused (tests/test-captive.sh). The other
appears to be an assertion failure bug in the new finalize code:
Thread 2 (Thread 0x7f12b820fa40 (LWP 231456)):
#0 0x00007f12b8783a1f in __GI___poll (fds=0x55771f18c550, nfds=nfds@entry=2, timeout=timeout@entry=-1) at ../sysdeps/unix/sysv/linux/poll.c:29
#1 0x00005577156b9646 in poll (__timeout=-1, __nfds=2, __fds=<optimized out>) at /usr/include/bits/poll2.h:46
#2 check_sockets_and_quit_fd (nr_socks=1, socks=0x55771f18c2e0) at sockets.c:466
#3 accept_incoming_connections (socks=0x55771f18c2e0, nr_socks=1) at sockets.c:494
#4 0x00005577156ac6ac in start_serving () at main.c:914
#5 main (argc=<optimized out>, argv=0x7ffd0bcb15d8) at main.c:685
Thread 1 (Thread 0x7f12b820e700 (LWP 231469)):
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f12b86b28d9 in __GI_abort () at abort.c:79
#2 0x00007f12b86b27a9 in __assert_fail_base (fmt=0x7f12b881db18 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x5577156bb09d "h->state & HANDLE_CONNECTED", file=0x5577156bb064 "backend.c", line=240, function=<optimized out>) at assert.c:92
#3 0x00007f12b86c1a66 in __GI___assert_fail (assertion=assertion@entry=0x5577156bb09d "h->state & HANDLE_CONNECTED", file=file@entry=0x5577156bb064 "backend.c", line=line@entry=240, function=function@entry=0x5577156bb880 <__PRETTY_FUNCTION__.6528> "backend_finalize") at assert.c:101
#4 0x00005577156ad04e in backend_finalize (b=<optimized out>, conn=conn@entry=0x55771f1ac710) at backend.c:240
#5 0x00005577156b6ea8 in negotiate_handshake_newstyle_options (conn=<optimized out>) at protocol-handshake-newstyle.c:484
#6 protocol_handshake_newstyle (conn=0x55771f1ac710) at protocol-handshake-newstyle.c:762
#7 0x00005577156b5705 in protocol_handshake (conn=conn@entry=0x55771f1ac710) at protocol-handshake.c:55
#8 0x00005577156af73a in handle_single_connection (sockin=<optimized out>, sockout=15) at connections.c:167
#9 0x00005577156b8a48 in start_thread (datav=0x55771f18c620) at sockets.c:356
#10 0x00007f12b885f4e2 in start_thread (arg=<optimized out>) at pthread_create.c:479
#11 0x00007f12b878e643 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
Rich.
4 years, 6 months