On Thu, Sep 07, 2023 at 11:35:27AM -0500, Eric Blake wrote: > 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.
I'll do a release. > > > > 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 Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
