On Tue, Sep 05, 2023 at 03:23:58PM +0100, Richard W.M. Jones wrote:
> On Tue, Sep 05, 2023 at 08:47:34AM -0500, Eric Blake wrote:
> > diff --git a/info/info-list-uris.sh b/info/info-list-uris.sh
> > index 0d6a16a0..d8ea9108 100755
> > --- a/info/info-list-uris.sh
> > +++ b/info/info-list-uris.sh
> > @@ -22,7 +22,14 @@ set -e
> >  set -x
> > 
> >  requires nbdkit --version
> > -requires nbdkit file --version
> > +# Avoid tickling a double-free bug in nbdkit 1.35.11
> > +requires nbdkit -U- -r file dir=. --run 'nbdsh --opt-mode -u "$uri" -c "
> > +try:
> > +  h.opt_info()
> > +except nbd.Error:
> > +  pass
> > +h.opt_abort()
> > +"'
> > 
> >  # This test requires nbdkit >= 1.22.
> 
> I guess the double-free bug is actually in any (or many versions of)
> nbdkit between 1.22 and 1.35.11, which is why the requires test is
> needed here?  The message seems to imply it only affects 1.35.11.

Hmm.  There is indeed a range of affected versions, but it is not as
bad as you make out.  The bug was introduced in 185e7d (v1.33.1) and
was not backported to stable-1.32; so if we assume distros only use
stable versions, then rejecting 1.34.[012] is sufficient. I've already
backported the fix to stable-1.34, but don't know if you would prefer
to cut the 1.34.3 release or let me do it.

> 
> It's a shame that the requires test is so long, but I couldn't see any
> way to shorten it.

And as you pointed out in the next message, triggering the double-free
can still have the side effect of a core file.  You've convinced me
that a version check is the best here.

> > @@ -398,6 +395,13 @@ do_connect (struct nbd_handle *nbd)
> >      fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
> >      exit (EXIT_FAILURE);
> >    }
> > +
> > +  /* If we are in opt mode, request info on the original export name.
> > +   * However, ignoring failure at this time is okay, as later code
> > +   * may want to try an alternate export name.
> > +   */
> > +  if (nbd_aio_is_negotiating (nbd))
> > +    nbd_opt_info (nbd);
> 
> I maybe don't fully understand when nbd_aio_is_negotiating would not
> be true.  Aren't we always in opt mode now?

We now unconditionally request it, but it is up to the server whether
we actually got it.  An oldstyle server does not support opt mode.
And our unit tests do have coverage of connecting to an oldstyle
server.

At any rate, with the revisions to patch 2 and 3 (nbd_shutdown now
automatically covers opt_abort without needing a new flag), and with
this patch updated to just do a version range exclusion, this series
is now in as db9f6bf6..0b587f8a

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to