Package: release.debian.org Severity: normal Tags: bookworm X-Debbugs-Cc: ost...@packages.debian.org Control: affects -1 + src:ostree src:flatpak libcurl3-gnutls User: release.debian....@packages.debian.org Usertags: pu
[ Reason ] Fix a serious regression in flatpak when libcurl3-gnutls is upgraded to the version from bookworm-backports [ Impact ] For users of pure bookworm, no impact. For users of bookworm-backports' libcurl3-gnutls, flatpak crashes with an assertion failure when trying to install or upgrade apps/runtimes, which is fixed by the proposed package. [ Tests ] Unfortunately neither the ostree test suite nor the flatpak test suite reproduces the assertion failure (they use a simple mock http server and the bad code path appears to require a fully-featured http server, possibly HTTP/2). There is a simple manual reproducer, and I've verified that the proposed package fixes the assertion failure here: $ podman run --rm -it debian:bookworm-slim # echo "deb http://deb.debian.org/debian bookworm-backports main" > /etc/apt/sources.list.d/debian-backports.list # apt update # apt install --no-install-recommends flatpak ca-certificates # apt install libcurl3-gnutls/bookworm-backports # flatpak remote-add --if-not-exists flathub https://dl.flathub.org/repo/flathub.flatpakrepo # flatpak install flathub org.gnome.Recipes (It is OK and expected that installation of org.freedesktop.Platform.openh264 fails with "apply_extra script failed", and other steps log a warning "bwrap: Creating new namespace failed: Operation not permitted", when installing inside an unprivileged container: this is a limitation of nested containers and is unrelated to the regression.) I have also confirmed that the proposed ostree version can successfully install a Flatpak app in a Debian 12 GNOME desktop VM with each of bookworm libcurl3-gnutls or bookworm-backports libcurl3-gnutls. [ Risks ] The actual fix seems very low-risk: it's a straightforward backport of an upstream change that they specifically called out as suitable for backporting. The accompanying change to add an assertion failure if curl_multi_assign() fails could conceivably make a wrong-but-harmless situation into a crash, but my understanding is that it's something that should never fail for reasons other than a programming error, and it does seem valuable to check this. [ Checklist ] [x] *all* changes are documented in the d/changelog [x] I reviewed all changes and I approve them [x] attach debdiff against the package in (old)stable [x] the issue is verified as fixed in unstable [ Changes ] d/control, d/gbp.conf: Administrivia because this is its first stable update in the bookworm cycle debian/patches/curl-Make-socket-callback-during-cleanup-into-no-op.patch: This is the actual bug-fix. The original uses C11 <stdbool.h>, but 2022.7 didn't use that, so I adjusted the patch to use GLib's booleans instead (the only functional difference is that the struct might be slightly larger). debian/patches/curl-Assert-that-curl_multi_assign-worked.patch: While debugging this assertion failure, upstream added an assertion that improved their ability to locate the problem. src/libostree/ostree-fetcher-curl.c: Modified by each of the patches, see above
diffstat for ostree-2022.7 ostree-2022.7 debian/changelog | 13 ++ debian/control | 2 debian/gbp.conf | 2 debian/patches/curl-Assert-that-curl_multi_assign-worked.patch | 31 ++++ debian/patches/curl-Make-socket-callback-during-cleanup-into-no-op.patch | 64 ++++++++++ debian/patches/series | 2 src/libostree/ostree-fetcher-curl.c | 17 ++ 7 files changed, 128 insertions(+), 3 deletions(-) diff -Nru ostree-2022.7/debian/changelog ostree-2022.7/debian/changelog --- ostree-2022.7/debian/changelog 2022-12-06 11:11:05.000000000 +0000 +++ ostree-2022.7/debian/changelog 2024-10-01 12:25:32.000000000 +0100 @@ -1,3 +1,16 @@ +ostree (2022.7-2+deb12u1) bookworm; urgency=medium + + * d/control, d/gbp.conf: Configure for stable updates + * d/p/curl-Assert-that-curl_multi_assign-worked.patch, + d/p/curl-Make-socket-callback-during-cleanup-into-no-op.patch: + Add patches from upstream 2024.8 to avoid libflatpak crash with an + assertion failure when using curl 8.10.x. + This was originally reported in testing/unstable, but can affect + bookworm if using libcurl3-gnutls from bookworm-backports. + (Closes: #1082121) + + -- Simon McVittie <s...@debian.org> Tue, 01 Oct 2024 12:25:32 +0100 + ostree (2022.7-2) unstable; urgency=medium * Skip test-sysroot.js on s390x (Mitigates: #1025532) diff -Nru ostree-2022.7/debian/control ostree-2022.7/debian/control --- ostree-2022.7/debian/control 2022-12-06 11:11:05.000000000 +0000 +++ ostree-2022.7/debian/control 2024-10-01 12:25:32.000000000 +0100 @@ -50,7 +50,7 @@ Rules-Requires-Root: no Standards-Version: 4.6.1 Homepage: https://github.com/ostreedev/ostree/ -Vcs-Git: https://salsa.debian.org/debian/ostree.git +Vcs-Git: https://salsa.debian.org/debian/ostree.git -b debian/bookworm Vcs-Browser: https://salsa.debian.org/debian/ostree Package: gir1.2-ostree-1.0 diff -Nru ostree-2022.7/debian/gbp.conf ostree-2022.7/debian/gbp.conf --- ostree-2022.7/debian/gbp.conf 2022-12-06 11:11:05.000000000 +0000 +++ ostree-2022.7/debian/gbp.conf 2024-10-01 12:25:32.000000000 +0100 @@ -1,7 +1,7 @@ [DEFAULT] pristine-tar = True compression = xz -debian-branch = debian/latest +debian-branch = debian/bookworm upstream-branch = upstream/latest patch-numbers = False upstream-vcs-tag = v%(version)s diff -Nru ostree-2022.7/debian/patches/curl-Assert-that-curl_multi_assign-worked.patch ostree-2022.7/debian/patches/curl-Assert-that-curl_multi_assign-worked.patch --- ostree-2022.7/debian/patches/curl-Assert-that-curl_multi_assign-worked.patch 1970-01-01 01:00:00.000000000 +0100 +++ ostree-2022.7/debian/patches/curl-Assert-that-curl_multi_assign-worked.patch 2024-10-01 12:25:32.000000000 +0100 @@ -0,0 +1,31 @@ +From: Colin Walters <walt...@verbum.org> +Date: Wed, 18 Sep 2024 13:21:27 -0400 +Subject: curl: Assert that curl_multi_assign worked + +ref https://github.com/ostreedev/ostree/issues/3299 + +This won't fix that issue, but *if* this assertion triggers +it should give us a better idea of the possible codepaths +where it is happening. + +Signed-off-by: Colin Walters <walt...@verbum.org> +Origin: upstream, 2024.8, commit:472d9d493a3e4a08415da4c337a7e831e0c5a5e2 +Bug-Debian: https://bugs.debian.org/1082121 +--- + src/libostree/ostree-fetcher-curl.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c +index 522eacf..3bbd9ba 100644 +--- a/src/libostree/ostree-fetcher-curl.c ++++ b/src/libostree/ostree-fetcher-curl.c +@@ -509,7 +509,8 @@ addsock (curl_socket_t s, CURL *easy, int action, OstreeFetcher *fetcher) + fdp->refcount = 1; + fdp->fetcher = fetcher; + setsock (fdp, s, action, fetcher); +- curl_multi_assign (fetcher->multi, s, fdp); ++ CURLMcode rc = curl_multi_assign (fetcher->multi, s, fdp); ++ g_assert_cmpint (rc, ==, CURLM_OK); + g_hash_table_add (fetcher->sockets, fdp); + } + diff -Nru ostree-2022.7/debian/patches/curl-Make-socket-callback-during-cleanup-into-no-op.patch ostree-2022.7/debian/patches/curl-Make-socket-callback-during-cleanup-into-no-op.patch --- ostree-2022.7/debian/patches/curl-Make-socket-callback-during-cleanup-into-no-op.patch 1970-01-01 01:00:00.000000000 +0100 +++ ostree-2022.7/debian/patches/curl-Make-socket-callback-during-cleanup-into-no-op.patch 2024-10-01 12:25:32.000000000 +0100 @@ -0,0 +1,64 @@ +From: Colin Walters <walt...@verbum.org> +Date: Wed, 18 Sep 2024 13:41:59 -0400 +Subject: curl: Make socket callback during cleanup into no-op + +Because curl_multi_cleanup may invoke callbacks, we effectively have +some circular references going on here. See discussion in + +https://github.com/curl/curl/issues/14860 + +Basically what we do is the socket callback libcurl may invoke into a no-op when +we detect we're finalizing. The data structures are owned by this object and +not by the callbacks, and will be destroyed below. Note that +e.g. g_hash_table_unref() may itself invoke callbacks, which is where +some data is cleaned up. + +Signed-off-by: Colin Walters <walt...@verbum.org> +Origin: upstream, 2024.8, commit:4d755a85225ea0a02d4580d088bb8a97138cb040 +Bug: https://github.com/ostreedev/ostree/issues/3299 +Bug-Debian: https://bugs.debian.org/1082121 +[smcv: Backport to 2022.7 by using gboolean instead of stdbool.h] +Signed-off-by: Simon McVittie <s...@debian.org> +--- + src/libostree/ostree-fetcher-curl.c | 14 ++++++++++++++ + 1 file changed, 14 insertions(+) + +diff --git a/src/libostree/ostree-fetcher-curl.c b/src/libostree/ostree-fetcher-curl.c +index 3bbd9ba..eae6c4a 100644 +--- a/src/libostree/ostree-fetcher-curl.c ++++ b/src/libostree/ostree-fetcher-curl.c +@@ -75,6 +75,7 @@ struct OstreeFetcher + char *proxy; + struct curl_slist *extra_headers; + int tmpdir_dfd; ++ gboolean finalizing; // Set if we're in the process of teardown + char *custom_user_agent; + + GMainContext *mainctx; +@@ -174,6 +175,15 @@ _ostree_fetcher_finalize (GObject *object) + { + OstreeFetcher *self = OSTREE_FETCHER (object); + ++ // Because curl_multi_cleanup may invoke callbacks, we effectively have ++ // some circular references going on here. See discussion in ++ // https://github.com/curl/curl/issues/14860 ++ // Basically what we do is make most callbacks libcurl may invoke into no-ops when ++ // we detect we're finalizing. The data structures are owned by this object and ++ // not by the callbacks, and will be destroyed below. Note that ++ // e.g. g_hash_table_unref() may itself invoke callbacks, which is where ++ // some data is cleaned up. ++ self->finalizing = TRUE; + curl_multi_cleanup (self->multi); + g_free (self->remote_name); + g_free (self->tls_ca_db_path); +@@ -521,6 +531,10 @@ sock_cb (CURL *easy, curl_socket_t s, int what, void *cbp, void *sockp) + OstreeFetcher *fetcher = cbp; + SockInfo *fdp = (SockInfo*) sockp; + ++ // We do nothing if we're in the process of teardown; see below. ++ if (fetcher->finalizing) ++ return 0; ++ + if (what == CURL_POLL_REMOVE) + { + if (!g_hash_table_remove (fetcher->sockets, fdp)) diff -Nru ostree-2022.7/debian/patches/series ostree-2022.7/debian/patches/series --- ostree-2022.7/debian/patches/series 2022-12-06 11:11:05.000000000 +0000 +++ ostree-2022.7/debian/patches/series 2024-10-01 12:25:32.000000000 +0100 @@ -1,3 +1,5 @@ configure-use-pkg-config-with-newer-gpgme-and-gpg-error.patch +curl-Assert-that-curl_multi_assign-worked.patch +curl-Make-socket-callback-during-cleanup-into-no-op.patch debian/Skip-test-pull-repeated-during-CI.patch debian/test-sysroot-Skip-on-s390x-by-default.patch diff -Nru ostree-2022.7/src/libostree/ostree-fetcher-curl.c ostree-2022.7/src/libostree/ostree-fetcher-curl.c --- ostree-2022.7/src/libostree/ostree-fetcher-curl.c 2022-10-29 23:03:49.000000000 +0100 +++ ostree-2022.7/src/libostree/ostree-fetcher-curl.c 2024-10-01 12:37:15.000000000 +0100 @@ -75,6 +75,7 @@ char *proxy; struct curl_slist *extra_headers; int tmpdir_dfd; + gboolean finalizing; // Set if we're in the process of teardown char *custom_user_agent; GMainContext *mainctx; @@ -174,6 +175,15 @@ { OstreeFetcher *self = OSTREE_FETCHER (object); + // Because curl_multi_cleanup may invoke callbacks, we effectively have + // some circular references going on here. See discussion in + // https://github.com/curl/curl/issues/14860 + // Basically what we do is make most callbacks libcurl may invoke into no-ops when + // we detect we're finalizing. The data structures are owned by this object and + // not by the callbacks, and will be destroyed below. Note that + // e.g. g_hash_table_unref() may itself invoke callbacks, which is where + // some data is cleaned up. + self->finalizing = TRUE; curl_multi_cleanup (self->multi); g_free (self->remote_name); g_free (self->tls_ca_db_path); @@ -509,7 +519,8 @@ fdp->refcount = 1; fdp->fetcher = fetcher; setsock (fdp, s, action, fetcher); - curl_multi_assign (fetcher->multi, s, fdp); + CURLMcode rc = curl_multi_assign (fetcher->multi, s, fdp); + g_assert_cmpint (rc, ==, CURLM_OK); g_hash_table_add (fetcher->sockets, fdp); } @@ -520,6 +531,10 @@ OstreeFetcher *fetcher = cbp; SockInfo *fdp = (SockInfo*) sockp; + // We do nothing if we're in the process of teardown; see below. + if (fetcher->finalizing) + return 0; + if (what == CURL_POLL_REMOVE) { if (!g_hash_table_remove (fetcher->sockets, fdp))
