[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1006905: bullseye-pu: package ostree/2020.8-2+deb11u1



Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: Dan Nicholson <dbn@endlessos.org>

This update isn't finalized yet, but does it look like something of a
scale that you would be likely to accept in a point release?

[ Reason ]
Backport several bug fixes from testing/unstable, prompted by a user
request in #1004467 and various issues reported to Flatpak upstream

[ Impact ]
In descending priority order:

If d/p/Fall-back-if-copy_file_range-fails-with-EINVAL.patch is not applied,
users with an eCryptFS encrypted home directory cannot use `flatpak --user`.
If they had already configured all necessary remotes before encrypting the
home directory, the symptom is that `flatpak build-bundle` crashes; if not,
from my tests on unstable, it appears that the symptom is that
`flatpak remote-add` fails. (#1004467)
There are indications from upstream bug reports that this patch might
also fix similar issues for ZFS users, although this is not yet confirmed.

If d/p/Fix-marking-static-delta-commits-as-partial.patch is not applied,
interrupted downloads can result in a corrupted local repository, requiring
manual cleanup via `flatpak repair` or `ostree fsck`.

If d/p/libotutil-Avoid-infinite-recursion-during-error-unwinding.patch is
not applied, some failure modes (in particular the symptom of #1004467)
lead to a crash instead of a graceful failure.

If d/p/Fix-translation-of-file-URIs-into-paths.patch is not applied,
users will be unable to install Flatpak apps from paths on the local
filesystem that contain a backslash or non-ASCII, most commonly during
"sideloading" from a USB stick created by `flatpak create-usb`.

If d/p/lib-Fix-a-bad-call-to-g_file_get_child.patch is not applied,
checking out only a subset of a commit (most frequently seen in Flatpak
when installing locale data) can fail with an assertion failure if a
backport or local build of GLib 2.71 is in use. I'm a little unsure about
this one for bullseye, since I would generally not recommend backporting
something as foundational as GLib, but it's a one-line fix. With my
GNOME team hat on, we need to get GLib >= 2.72 into bookworm within the
next few weeks, so if someone does a backport of bookworm's GLib into
bullseye-backports, then the priority of this change will suddenly go up.

[ Tests ]
Automated tests are run at build-time and via autopkgtest, and pass.
Build-time and ci.debian.net coverage are limited by the buildd and
debci testbed using schroot/lxc, which prevents some of the more exotic
scenarios from being tested, but I run tests on an amd64 qemu VM before
each upload and those also pass.

I will give this some real-world testing via Flatpak on bullseye
before upload (in particular to confirm that the eCryptFS fix has been
successful), but so far it has only had automated tests.

[ Risks ]
I would say this is low-risk. They're narrowly-targeted patches, and
all are straightforward backports, either from upstream release 2022.2
(in unstable, not in testing yet) or from older releases that are already
in testing.

If one of these changes does cause a regression, then it's most likely to
be something similar to the bugs I'm fixing: a crash in a very specific
scenario that most Flatpak users don't exercise.

[ 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 ]
See d/changelog, the Impact description above, and the individual patches
for details.

[ Other info ]
I'm open to suggestions for additional patches with a similar risk/impact
ratio. I've cc'd Dan Nicholson, who maintains the fork of our Flatpak and
libostree packages in Endless, a Debian derivative that makes very heavy use
of both Flatpak and libostree.
diff --git a/debian/changelog b/debian/changelog
index 3e4cd1b0..0bd98941 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,30 @@
+ostree (2020.8-2+deb11u1) UNRELEASED; urgency=medium
+
+  * d/gbp.conf, d/control: Branch for Debian 11 updates
+  * Backport various bug fixes from newer libostree releases.
+    Each of these fixes an issue that was reported against Flatpak when
+    using the libostree from Debian 11, either via bullseye or
+    buster-backports.
+    - d/p/Fall-back-if-copy_file_range-fails-with-EINVAL.patch:
+      Add patch to fall back if copy_file_range fails with EINVAL.
+      This fixes an incompatibility with eCryptFS, in particular when
+      using Flatpak in an eCryptFS home directory. (Closes: #1004467)
+    - d/p/libotutil-Avoid-infinite-recursion-during-error-unwinding.patch:
+      Avoid infinite recursion when recovering from certain errors, in
+      particular the one that was a symptom of #1004467.
+    - d/p/Fix-marking-static-delta-commits-as-partial.patch:
+      Mark commits as partial before downloading, to avoid Flatpak and other
+      ostree users getting into a state where a failed download cannot be
+      resumed.
+    - d/p/lib-Fix-a-bad-call-to-g_file_get_child.patch:
+      Fix an assertion failure when using a backport or local build of
+      GLib >= 2.71
+    - d/p/Fix-translation-of-file-URIs-into-paths.patch:
+      Fix the ability to fetch OSTree content from paths containing
+      non-URI characters (such as backslashes) or non-ASCII
+
+ -- Simon McVittie <smcv@debian.org>  Mon, 07 Mar 2022 23:37:37 +0000
+
 ostree (2020.8-2) unstable; urgency=medium
 
   * d/p/test-pull-summary-sigs-Set-timestamps-to-serve-expected-f.patch:
diff --git a/debian/control b/debian/control
index 14c73f5a..9c0c688b 100644
--- a/debian/control
+++ b/debian/control
@@ -49,7 +49,7 @@ Build-Depends-Indep:
 Rules-Requires-Root: no
 Standards-Version: 4.5.0
 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/bullseye
 Vcs-Browser: https://salsa.debian.org/debian/ostree
 
 Package: gir1.2-ostree-1.0
diff --git a/debian/gbp.conf b/debian/gbp.conf
index 2bda9c8b..7cbb2ecb 100644
--- a/debian/gbp.conf
+++ b/debian/gbp.conf
@@ -1,7 +1,7 @@
 [DEFAULT]
 pristine-tar = True
 compression = xz
-debian-branch = debian/master
+debian-branch = debian/bullseye
 upstream-branch = upstream/latest
 patch-numbers = False
 upstream-vcs-tag = v%(version)s
diff --git a/debian/patches/Fall-back-if-copy_file_range-fails-with-EINVAL.patch b/debian/patches/Fall-back-if-copy_file_range-fails-with-EINVAL.patch
new file mode 100644
index 00000000..e9705c31
--- /dev/null
+++ b/debian/patches/Fall-back-if-copy_file_range-fails-with-EINVAL.patch
@@ -0,0 +1,31 @@
+From: Olaf Leidinger <oleid@mescharet.de>
+Date: Mon, 24 Jan 2022 16:12:57 +0100
+Subject: Fall back if copy_file_range fails with EINVAL
+
+Although EINVAL usually indicates a programming error, ecryptfs (and
+possibly other stacked filesystems) returns EINVAL for attempts to
+copy_file_range() or sendfile() between files on that filesystem.
+
+Bug: https://gitlab.gnome.org/GNOME/libglnx/-/issues/3
+Bug: https://github.com/ostreedev/ostree/issues/2525
+Bug: https://github.com/flatpak/flatpak/issues/4673
+Bug-Debian: https://bugs.debian.org/1004467
+Applied-upstream: libglnx commit:24231a956a4b849087fbf01173cdebb53e1bd60b
+Applied-upstream: libostree 2022.2, commit:0ebf9d9f64647d0b8b95cda112c7854be6f58ed3
+---
+ libglnx/glnx-fdio.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/libglnx/glnx-fdio.c b/libglnx/glnx-fdio.c
+index 422bc2d..0bb26f4 100644
+--- a/libglnx/glnx-fdio.c
++++ b/libglnx/glnx-fdio.c
+@@ -829,7 +829,7 @@ glnx_regfile_copy_bytes (int fdf, int fdt, off_t max_bytes)
+                   have_cfr = 0;
+                   try_cfr = false;
+                 }
+-              else if (G_IN_SET (errno, EXDEV, EOPNOTSUPP))
++              else if (G_IN_SET (errno, EXDEV, EINVAL, EOPNOTSUPP))
+                 /* We won't try cfr again for this run, but let's be
+                  * conservative and not mark it as available/unavailable until
+                  * we know for sure.
diff --git a/debian/patches/Fix-marking-static-delta-commits-as-partial.patch b/debian/patches/Fix-marking-static-delta-commits-as-partial.patch
new file mode 100644
index 00000000..4fe264e0
--- /dev/null
+++ b/debian/patches/Fix-marking-static-delta-commits-as-partial.patch
@@ -0,0 +1,60 @@
+From: Phaedrus Leeds <mwleeds@protonmail.com>
+Date: Sat, 19 Feb 2022 07:55:02 -0600
+Subject: Fix marking static delta commits as partial
+
+This patch makes it so that we mark the .commit file from a static delta
+as partial before writing the commit to the staging directory. This
+exactly mirrors what we do in meta_fetch_on_complete() when writing the
+commit on that codepath, which should lend some credibility to the
+correctness of this patch.
+
+I have checked that this fixes an issue Flatpak users have been
+encountering (https://github.com/flatpak/flatpak/issues/3479) which
+results in error messages like "error: Failed to install
+org.freedesktop.Sdk.Extension.texlive: Failed to read commit
+c7958d966cfa8b80a42877d1d6124831d7807f93c89461a2a586956aa28d438a: No
+such metadata object
+8bdaa943b957f3cf14d19301c59c7eec076e57389e0fbb3ef5d30082e47a178f.dirtree"
+
+Here's the sequence of events that lead to the error:
+1. An install operation is started that fetches static deltas.
+2. The fetch is interrupted for some reason such as network connectivity
+   dropping.
+3. The .commit and .commitmeta files for the commit being pulled are
+   left in the staging dir, e.g.
+   "~/.local/share/flatpak/repo/tmp/staging-dfe862b2-13fc-49a2-ac92-5a59cc0d8e18-RURckd"
+4. There is no `.commitpartial` file for the commit in
+   "~/.local/share/flatpak/repo/state/"
+5. The next time the user attempts the install, libostree reuses the
+   existing staging dir, pulls the commit and commitmeta objects into
+   the repo from the staging dir on the assumption that it's a complete
+   commit.
+6. Flatpak then tries to deploy the commit but fails in
+   ostree_repo_read_commit() in flatpak_dir_deploy(), leading to the
+   error message "Failed to read commit ..."
+7. This happens again any subsequent time the user attempts the install,
+   until the incomplete commit is removed with "flatpak repair --user".
+
+I will try to also add a workaround in Flatpak so this is fixed even
+when Flatpak links against affected versions of libostree.
+
+Bug: https://github.com/flatpak/flatpak/issues/3479
+Origin: upstream, 2022.2, commit:5d3b1ca37a508e9f80702b7ef7383fe95253ec6a
+---
+ src/libostree/ostree-repo-pull.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
+index 758c505..4f44191 100644
+--- a/src/libostree/ostree-repo-pull.c
++++ b/src/libostree/ostree-repo-pull.c
+@@ -2214,6 +2214,9 @@ process_one_static_delta (OtPullData                 *pull_data,
+                                          ref, cancellable, error))
+             return FALSE;
+ 
++          if (!ostree_repo_mark_commit_partial (pull_data->repo, to_revision, TRUE, error))
++            return FALSE;
++
+           if (detached_data && !ostree_repo_write_commit_detached_metadata (pull_data->repo,
+                                                                             to_revision,
+                                                                             detached_data,
diff --git a/debian/patches/Fix-translation-of-file-URIs-into-paths.patch b/debian/patches/Fix-translation-of-file-URIs-into-paths.patch
new file mode 100644
index 00000000..10622811
--- /dev/null
+++ b/debian/patches/Fix-translation-of-file-URIs-into-paths.patch
@@ -0,0 +1,57 @@
+From: Phaedrus Leeds <mwleeds@endlessos.org>
+Date: Wed, 10 Mar 2021 10:02:14 -0800
+Subject: Fix translation of file:// URIs into paths
+
+Currently if a file path contains a special character such as '\', and
+that character is encoded into a file:// URI that is passed to
+ostree_repo_pull_with_options(), the percent encoding will remain in the
+path passed to g_file_new() (in the case of backslash %5C) and the pull
+will then fail with a file not found error. This is an important edge
+case to handle because by default on many Linux distributions a
+filesystem with no label is mounted at a path based on its UUID, and
+this is then passed to systemd-escape by Flatpak (when
+--enable-auto-sideloading was used at compile time) to create a symbolic
+link such as this which contains backslashes:
+
+$ ls -l /run/flatpak/sideload-repos/
+total 0
+lrwxrwxrwx 1 mwleeds mwleeds 55 Mar  9 14:21
+'automount-run-media-mwleeds-29419e8f\x2dc680\x2d4e95\x2d9a31\x2d2cc907d421cb'
+-> /run/media/mwleeds/29419e8f-c680-4e95-9a31-2cc907d421cb
+
+And Flatpak then passes libostree a file:// URI containing that path, to
+implement sideloading (pulling content from the USB drive).
+
+This results in an error like:
+
+Error: While pulling app/org.videolan.VLC/x86_64/stable from remote
+flathub:
+/run/flatpak/sideload-repos/automount-run-media-mwleeds-29419e8f%5Cx2dc680%5Cx2d4e95%5Cx2d9a31%5Cx2d2cc907d421cb/.ostree/repo:
+opendir(/run/flatpak/sideload-repos/automount-run-media-mwleeds-29419e8f%5Cx2dc680%5Cx2d4e95%5Cx2d9a31%5Cx2d2cc907d421cb/.ostree/repo):
+No such file or directory
+
+This patch avoids such errors by using g_file_new_for_uri() instead of
+g_file_new_for_path(), so that GLib handles the %-decoding for us.
+
+Bug: https://community.endlessos.com/t/can-not-install-vlc-from-usb-drive-3-9-3/16353
+Bug: https://github.com/flatpak/flatpak/issues/4378
+Origin: upstream, 2021.1, commit:19577522f8eacd868cf25d53e1ac0e7f424e952b
+---
+ src/libostree/ostree-repo-pull.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
+index 4f44191..ac9e7a7 100644
+--- a/src/libostree/ostree-repo-pull.c
++++ b/src/libostree/ostree-repo-pull.c
+@@ -4084,8 +4084,8 @@ ostree_repo_pull_with_options (OstreeRepo             *self,
+    */
+   if (g_str_equal (first_scheme, "file") && !pull_data->require_static_deltas)
+     {
+-      g_autofree char *path = _ostree_fetcher_uri_get_path (first_uri);
+-      g_autoptr(GFile) remote_repo_path = g_file_new_for_path (path);
++      g_autofree char *uri = _ostree_fetcher_uri_to_string (first_uri);
++      g_autoptr(GFile) remote_repo_path = g_file_new_for_uri (uri);
+       pull_data->remote_repo_local = ostree_repo_new (remote_repo_path);
+       if (!ostree_repo_open (pull_data->remote_repo_local, cancellable, error))
+         goto out;
diff --git a/debian/patches/lib-Fix-a-bad-call-to-g_file_get_child.patch b/debian/patches/lib-Fix-a-bad-call-to-g_file_get_child.patch
new file mode 100644
index 00000000..be2c10bd
--- /dev/null
+++ b/debian/patches/lib-Fix-a-bad-call-to-g_file_get_child.patch
@@ -0,0 +1,39 @@
+From: Valentin David <me@valentindavid.com>
+Date: Tue, 2 Nov 2021 19:49:04 +0100
+Subject: lib: Fix a bad call to g_file_get_child
+
+In Glib, since commit 3a6e8bc8876e149c36b6b14c6a25a718edb581ed,
+`g_file_get_child` does not accept absolute path as paramater anymore.
+
+The broken assertion was encountered during `ostree admin deploy`
+command for the checkout of subpath `etc`.
+
+Example of error log:
+```
+(ostree admin deploy:1640): GLib-GIO-CRITICAL **: 03:42:00.570: g_file_get_child: assertion '!g_path_is_absolute (name)' failed
+
+(ostree admin deploy:1640): GLib-GIO-CRITICAL **: 03:42:00.570: g_file_query_info: assertion 'G_IS_FILE (file)' failed
+**
+OSTree:ERROR:src/ostree/ot-main.c:232:ostree_run: assertion failed: (success || error)
+Bail out! OSTree:ERROR:src/ostree/ot-main.c:232:ostree_run: assertion failed: (success || error)
+```
+
+Bug: https://github.com/flatpak/flatpak/pull/4707
+Origin: upstream, 2021.6, commit:adc097a2edb1b7aaf5604043b4b1d5bd6ef8a308
+---
+ src/libostree/ostree-repo-checkout.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
+index 00c6a77..c6645d7 100644
+--- a/src/libostree/ostree-repo-checkout.c
++++ b/src/libostree/ostree-repo-checkout.c
+@@ -1381,7 +1381,7 @@ ostree_repo_checkout_at (OstreeRepo                        *self,
+   g_autoptr(GFile) target_dir = NULL;
+ 
+   if (strcmp (options->subpath, "/") != 0)
+-    target_dir = g_file_get_child (commit_root, options->subpath);
++    target_dir = g_file_resolve_relative_path (commit_root, options->subpath);
+   else
+     target_dir = g_object_ref (commit_root);
+   g_autoptr(GFileInfo) target_info =
diff --git a/debian/patches/libotutil-Avoid-infinite-recursion-during-error-unwinding.patch b/debian/patches/libotutil-Avoid-infinite-recursion-during-error-unwinding.patch
new file mode 100644
index 00000000..55665d37
--- /dev/null
+++ b/debian/patches/libotutil-Avoid-infinite-recursion-during-error-unwinding.patch
@@ -0,0 +1,36 @@
+From: Simon McVittie <smcv@collabora.com>
+Date: Fri, 28 Jan 2022 11:08:00 +0000
+Subject: libotutil: Avoid infinite recursion during error unwinding
+
+When we clean up from an error, for example copy_file_range() failing
+while we generate a static delta (perhaps caused by
+https://gitlab.gnome.org/GNOME/libglnx/-/issues/3 or by a
+genuine write error), we might free a variant builder that has a
+non-null parent. Previously, this caused infinite recursion and a stack
+overflow, repeatedly freeing the same object, but Luca Bruno suggested
+that the intention here appears to have been to free the parent object.
+
+Partially resolves https://github.com/ostreedev/ostree/issues/2525
+(the other bug reported in that issue needs to be resolved by updating
+libglnx to a version where libglnx#3 has been fixed).
+
+Signed-off-by: Simon McVittie <smcv@collabora.com>
+Bug: https://github.com/ostreedev/ostree/issues/2525
+Applied-upstream: 2022.2, commit:920f85cabc656e4a7c07574aa9af211b6153756d
+---
+ src/libotutil/ot-variant-builder.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/src/libotutil/ot-variant-builder.c b/src/libotutil/ot-variant-builder.c
+index 6636068..afbdc68 100644
+--- a/src/libotutil/ot-variant-builder.c
++++ b/src/libotutil/ot-variant-builder.c
+@@ -832,7 +832,7 @@ static void
+ ot_variant_builder_info_free (OtVariantBuilderInfo *info)
+ {
+   if (info->parent)
+-    ot_variant_builder_info_free (info);
++    ot_variant_builder_info_free (info->parent);
+ 
+   g_variant_type_free (info->type);
+   g_array_unref (info->child_ends);
diff --git a/debian/patches/series b/debian/patches/series
index 0070b994..b2fc9c44 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,7 @@
 test-pull-summary-sigs-Set-timestamps-to-serve-expected-f.patch
+Fall-back-if-copy_file_range-fails-with-EINVAL.patch
+libotutil-Avoid-infinite-recursion-during-error-unwinding.patch
+Fix-marking-static-delta-commits-as-partial.patch
+lib-Fix-a-bad-call-to-g_file_get_child.patch
+Fix-translation-of-file-URIs-into-paths.patch
 debian/Skip-test-pull-repeated-during-CI.patch

Reply to: