Package: release.debian.org
Severity: normal
Tags: bookworm
User: release.debian@packages.debian.org
Usertags: pu
X-Debbugs-Cc: repre...@packages.debian.org, b...@debian.org
Control: affects -1 + src:reprepro
reprepro uses internal decompressors for most compression formats except
zstd. When dealing with zstd compressed .debs (such as those for
Ubuntu), decompression may fail due to a race condition. Naturally, this
bug originally surfaced in Ubuntu as
https://bugs.launchpad.net/ubuntu/+source/reprepro/+bug/2008508. While
originally, it seemed only reproducible on s390x, it turned out that if
you generate Contents indices, it is more widely reproducible. I can
reliably reproduce it on Freexian infrastructure and filed #1050321.
[ Reason ]
The condition for reproducing the issue seem sufficiently common:
* Interact with zstd-compressed .debs
* Enable Contents indices
[ Impact ]
Once a zstd compressed .deb is included, most interactions leave
messages such as the following and Contents indices are incomplete.
zstd: /*stdout*\: Broken pipe
reading data.tar within
'./pool/main/p/php7.0/php7.0_7.0.33-67+freexian22.04.1+php+1_all.deb' failed:
/usr/bin/unzstd exited with code 1
[ Tests ]
Simon Chopin, who originally fixed the bug in Ubuntu, was only able to
reproduce it with a wrapper to unzstd. By enabling Contents indices, I
was able to reproduce it reliably on amd64 both in bookworm and
unstable. Once updating to experimental (where the Simon's MR is
merged), the issue went away. I've also deployed the proposed update to
the affected Freexian server and the issue is gone there as well.
[ Risks ]
When not dealing with zstd-compressed .debs, the patched code paths are
not exercised. Therefore, there only really is any risk for people
interacting with zstd-compressed .debs.
[ 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
[ ] the issue is verified as fixed in unstable
-> The issue is only fixed in experimental as of this writing, but
the maintainer intends to upload to unstable and Ubuntu uses a
very similar backport.
[ Changes ]
I think Simon Chopin does a much better job in is git commit message
than I could do here. The message is included in the .debdiff
[ Other info ]
The maintainer agreed to me doing the SPU.
Thanks for considering
Helmut
diff --minimal -Nru reprepro-5.3.1/debian/changelog
reprepro-5.3.1/debian/changelog
--- reprepro-5.3.1/debian/changelog 2022-07-19 19:00:04.0 +0200
+++ reprepro-5.3.1/debian/changelog 2023-08-23 12:33:31.0 +0200
@@ -1,3 +1,14 @@
+reprepro (5.3.1-1+deb12u1) bookworm; urgency=medium
+
+ * Upload to stable with ack from maintainer.
+
+ [ Simon Chopin ]
+ * d/p/0001-uncompress-wait-until-the-child-as-exited-to-close-t.patch:
+Fix a race condition when using external decompressors
+(Closes: #1050321, LP: #2008508)
+
+ -- Helmut Grohne Wed, 23 Aug 2023 12:33:31 +0200
+
reprepro (5.3.1-1) unstable; urgency=medium
* Update debhelper-compat to level 12
diff --minimal -Nru
reprepro-5.3.1/debian/patches/0001-uncompress-wait-until-the-child-as-exited-to-close-t.patch
reprepro-5.3.1/debian/patches/0001-uncompress-wait-until-the-child-as-exited-to-close-t.patch
---
reprepro-5.3.1/debian/patches/0001-uncompress-wait-until-the-child-as-exited-to-close-t.patch
1970-01-01 01:00:00.0 +0100
+++
reprepro-5.3.1/debian/patches/0001-uncompress-wait-until-the-child-as-exited-to-close-t.patch
2023-08-23 12:33:05.0 +0200
@@ -0,0 +1,149 @@
+From 1b5a3c557b1ae842ee95b6a7e333046e56632559 Mon Sep 17 00:00:00 2001
+From: Simon Chopin
+Date: Wed, 1 Mar 2023 17:53:28 +0100
+Subject: [PATCH] uncompress: wait until the child as exited to close the pipe
+
+Since we sometimes interrupt the decompression mid-stream as we're only
+looking for specific data, e.g. the control file in control.tar.zstd, it
+can happen that the child process still has a backlog of data to write
+out to the pipes before exiting. If we close its stdout pipe before
+calling waitpid(), it's going to encounter an EPIPE rather than
+gracefully exit.
+
+The fix is to only close the pipe fd after waitpid() successfully exits.
+
+However, that introduces a new theoretical issue: the child process could
+be blocking while writing into its stdout if the pipe is full, thus
+leading to a deadlock. To avoid this, we have to drain the pipe before
+waiting. Technically this should probably be done in a loop, but since
+it's fairly unlikely to be blocked on stdout in the first place, having
+enough pending data to fill the pipe *twice* seems too rare to bother
+with in the first place.
+
+The initial problem has first been noticed in Ubuntu autopkgtests on
+s390x when upgrading to libarchive 3.6.2, where unzstd would loudly
+complain about an -EPIPE (Ubuntu is using zstd as its default
