Libguestfs - Feb 2022 - [PATCH nbdkit] tls-fallback: Always call next open function

This fixes a crash when using --filter=tls-fallback.  It happens to
only occur with my block_size patch series and is easily reproducible
(see commit message).  I believe the problem is generic although I
couldn't work out how to trigger in unpatched nbdkit.

tls-fallback filter has a scary comment which seems to indicate that
calling the next open is wrong (unsafe?) in the !tls case.  So I don't
know if this change is really right.

Rich.

$ NBDKIT_GDB=1 ./nbdkit -fv --filter=tls-fallback memory 1m
(gdb) run

In a second window:

$ nbdinfo nbd://localhost

nbdkit crashes:

Thread 2 "nbdkit" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff7584640 (LWP 1129108)]
0x000000000040625d in backend_block_size (c=0x0, minimum=0x7ffff7583944,
preferred=0x7ffff7583940, maximum=0x7ffff758393c) at backend.c:428
428	  struct backend *b = c->b;

It turns out this is because the filter + plugin context chain is
incorrectly constructed.  There is only a single layer (for the
tls-fallback filter), and this has c->c_next == NULL.

We end up calling backend_block_size (c=NULL, ...) which causes the
crash above.

This is because the filter doesn't always call the next open function.
Changing the filter so it always calls it fixes the issue.
---
 filters/tls-fallback/tls-fallback.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/filters/tls-fallback/tls-fallback.c
b/filters/tls-fallback/tls-fallback.c
index fab9e58b..4ea48d09 100644
--- a/filters/tls-fallback/tls-fallback.c
+++ b/filters/tls-fallback/tls-fallback.c
@@ -106,13 +106,10 @@ tls_fallback_open (nbdkit_next_open *next, nbdkit_context
*nxdata,
                    int readonly,
                    const char *exportname, int is_tls)
 {
-  /* We do NOT want to call next() when insecure, because we don't
-   * know how long it will take.
-   */
-  if (!is_tls)
-    return &message; /* See NOT_TLS for this choice of handle */
   if (next (nxdata, readonly, exportname) == -1)
     return NULL;
+  if (!is_tls)
+    return &message; /* See NOT_TLS for this choice of handle */
   return NBDKIT_HANDLE_NOT_NEEDED;
 }
 
-- 
2.35.1

On Wed, Feb 16, 2022 at 09:25:50PM +0000, Richard W.M. Jones
wrote:
> This fixes a crash when using --filter=tls-fallback.  It happens to
> only occur with my block_size patch series and is easily reproducible
> (see commit message).  I believe the problem is generic although I
> couldn't work out how to trigger in unpatched nbdkit.
> 
> tls-fallback filter has a scary comment which seems to indicate that
> calling the next open is wrong (unsafe?) in the !tls case.  So I don't
> know if this change is really right.
Ignore this patch, see second one just posted.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW