| Summary: | GNOME tarballs with 99+ char filenames cause troubles with app-arch/tar-1.21+ when piped into | ||
|---|---|---|---|
| Product: | Portage Development | Reporter: | Doug Goldstein (RETIRED) <cardoe> |
| Component: | Core - Ebuild Support | Assignee: | Gentoo Linux Gnome Desktop Team <gnome> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | amigadave, base-system, carlo, chaseguard, esigra, ferringb, matrix47, notordoktor, pms, truedfx, zeev.tarantov |
| Priority: | High | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | https://bugzilla.gnome.org/show_bug.cgi?id=613808 | ||
| See Also: |
https://bugs.gentoo.org/show_bug.cgi?id=675828 https://bugs.gentoo.org/show_bug.cgi?id=887817 |
||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | 252680, 346819 | ||
| Bug Blocks: | 338744 | ||
| Attachments: |
tar-1.21-revert-pipe.patch.diff
tar-1.21-revert-pipe.patch.diff make portage trust tar exit status (ignore decompressor exit status) portage-2.2-sigpipe.patch handle SIGPIPE from decompressor handle SIGPIPE from decompressor |
||
probably Bug 252680 again since i dropped the patch this is going to keep coming up again and again unless i can put together a test case that reproduces the behavior and show it to upstream Created attachment 223165 [details, diff]
tar-1.21-revert-pipe.patch.diff
Original patch needs slight adjustment.
You're right spanky. It's the same problem again. Applying the modified patch makes the error disappear.
Created attachment 223167 [details, diff]
tar-1.21-revert-pipe.patch.diff
I should really be more accurate in reviewing my own patches *sigh*
Just another adjustment to make use of FATAL_ERROR
we can apply the patch and forget, but we really need to spend the time to put together a test case. i tested a few gnome packages myself with tar-1.23 and didnt hit any problems on my systems. What I find strange is that I cannot reproduce the error when I manually do the unpack like portage would do it: lars@barcelona:~/temp/tar_fail> bzip2 -dc $(portageq envvar DISTDIR)/evolution-data-server-2.28.3.1.tar.bz2 | tar xof - || echo fail lars@barcelona:~/temp/tar_fail> But I can 100% reproduce the problem with gnome-extra/evolution-data-server when unpacking it with /usr/bin/ebuild: barcelona:~ # ebuild $(portageq envvar PORTDIR)/gnome-extra/evolution-data-server/evolution-data-server-2.28.3.1.ebuild clean unpack &>/dev/null || echo fail fail barcelona:~ # Probably unrelated, but just in case: a story about a bug in python causing a problem when gzip is run in a subprocess to extract a file from a tarball. Might explain or give ideas about the difference between invoking extraction from command lines vs. from python. http://blog.nelhage.com/archives/150 Actually, that may very well be related:
$ python -c 'import os; os.system("bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2")' | tar -xf -
bzip2: I/O or other error, bailing out. Possible reason follows.
bzip2: Broken pipe
Input file = /usr/portage/distfiles/glibmm-2.18.1.tar.bz2, output file = (stdout)
$ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | tar -xf -
$
I now think it's much simpler, and that the above was misleading.
$ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | tar xf -
$ echo ${PIPESTATUS[*]}
141 0
$ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | ( tar xf -; tar xf - )
$ echo ${PIPESTATUS[*]}
0 0
glibmm-2.18.1.tar.bz2 is strange: after decompression, it's two concatenated tar files. bzip2 | tar, with tar 1.23, apparently makes tar exit as soon as it's read the first archive, and makes bzip2 exit with SIGPIPE. This is not a problem; in normal shells, bzip2's exit status would be ignored. portage, however, calls assert to check $PIPESTATUS[*], which includes bzip2's exit status. It should probably ignore that. If tar succeeds, any failure from bzip2 can't have been too important.
So, reassigning to the portage folks... Created attachment 223235 [details, diff]
make portage trust tar exit status (ignore decompressor exit status)
With this patch applied I get the following message from bzip2: # ebuild $(portageq envvar PORTDIR)/gnome-extra/evolution-data-server/evolution-data-server-2.28.3.1.ebuild clean unpack * evolution-data-server-2.28.3.1.tar.bz2 RMD160 SHA1 SHA256 size ;-) ...[ ok ] * checking ebuild checksums ;-) ... [ ok ] * checking auxfile checksums ;-) ... [ ok ] * checking miscfile checksums ;-) ... [ ok ] * checking evolution-data-server-2.28.3.1.tar.bz2 ;-) ... [ ok ] * CPV: gnome-extra/evolution-data-server-2.28.3.1 * REPO: gentoo * USE: amd64 elibc_glibc kernel_linux multilib ssl userland_GNU >>> Unpacking source... >>> Unpacking evolution-data-server-2.28.3.1.tar.bz2 to /var/tmp/portage/gnome-extra/evolution-data-server-2.28.3.1/work bzip2: I/O or other error, bailing out. Possible reason follows. bzip2: Broken pipe Input file = /var/tmp/portage/gnome-extra/evolution-data-server-2.28.3.1/distdir/evolution-data-server-2.28.3.1.tar.bz2, output file = (stdout) >>> Source unpacked in /var/tmp/portage/gnome-extra/evolution-data-server-2.28.3.1/work *** Bug 308993 has been marked as a duplicate of this bug. *** Created attachment 223303 [details, diff] portage-2.2-sigpipe.patch (In reply to comment #11) > With this patch applied I get the following message from bzip2: Now that looks more like what Zeev posted. Could you please try with this, on top of Zac's patch? emerge already resets SIGPIPE, but it calls ebuild, which doesn't. (In reply to comment #13) > Created an attachment (id=223303) [details] > portage-2.2-sigpipe.patch Thanks, that makes bzip2 get quietly killed. It's in svn r15823. Adding pms-bugs alias to CC since this can potentially affect unpack() implementations in all package managers. Uh. Hang on. Why are we adding in hacks in the package manager to make it not fail with broken inputs? I guess the definition of "broken" is debatable. If we can tweak unpack() to handle stuff like this, then we won't have to hear about it every time some upstream releases a "broken" tar file. Note that this seems to be an ongoing issue with tar (see bug 252680). Please explain how a bz2 file that bzip2 can't extract isn't broken. Bear in mind that you can quite often shove a truncated tar file through tar and it'll quite happily extract it, giving only a partial unpack, so long as you truncate the file at roughly the right place. Shrug, I have no preference on this issue. Any way we go is fine by me. (In reply to comment #19) > Shrug, I have no preference on this issue. Any way we go is fine by me. I'm pretty vehemently opposed to this change- as mentioned in IRC and reiterated by ciaran, tar can't be trusted in all cases. If the decompressor gets sigpipe, or stream ends before logical end of stream has been reached then the unpack failed as far as I'm concerned. Ok, the portage changes are reverted in svn r15827. (In reply to comment #18) > Please explain how a bz2 file that bzip2 can't extract isn't broken. > > Bear in mind that you can quite often shove a truncated tar file through tar > and it'll quite happily extract it, giving only a partial unpack, so long as > you truncate the file at roughly the right place. It's the other way around (for at least one of the files): the whole thing is a valid bzip2 file, but after decompression, the tar file has garbage at the end. There is no missing data, and if you ignore the SIGPIPE, both bzip2 and tar would have exited successfully. Truncated archives would make tar exit with a non-zero status, so would still be caught by the bzip2 | tar || die. Oh wait... if you're saying tar can exit successfully on a partial archive, then I agree, that's bad... For what it's worth, no error gets reported when using tar xjf. (In reply to comment #22) > It's the other way around (for at least one of the files): the whole thing is a > valid bzip2 file, but after decompression, the tar file has garbage at the end. Meaning the tarfile is corrupt- tar doesn't have a central header, it literally is archive record (specifying size, roughly), data, up to 512 bytes spacing to align on 512 boundaries, archive record, data, etc. Point I'm making there is that if there is data riding on the end of it, it's a *corrupt* tarball. gnutar may half ass ignore it, but try it w/ other stricter implementations- libarchive for example. (In reply to comment #24) > Point I'm making there is that if there is data riding on the end of it, it's a > *corrupt* tarball. gnutar may half ass ignore it, but try it w/ other stricter > implementations- libarchive for example. libarchive, busybox, and pax all ignore the garbage at the end. $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | tar xf - || echo tar reported failure $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | bsdtar xf - || echo tar reported failure $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | busybox tar xf - || echo tar reported failure $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | pax -r || echo tar reported failure $ (In reply to comment #25) > (In reply to comment #24) > > Point I'm making there is that if there is data riding on the end of it, it's a > > *corrupt* tarball. gnutar may half ass ignore it, but try it w/ other stricter > > implementations- libarchive for example. > > libarchive, busybox, and pax all ignore the garbage at the end. > > $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | tar xf - || echo tar > reported failure > $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | bsdtar xf - || echo > tar reported failure > $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | busybox tar xf - || > echo tar reported failure > $ bzip2 -dc /usr/portage/distfiles/glibmm-2.18.1.tar.bz2 | pax -r || echo tar > reported failure > $ Pick something other than glibmm- there is something definitely wrong w/ that tarball. If in doubt- mkdir tmp{1,2} tar jxf glibmm-2.18.1.tar.bz2 -C tmp1 bzip2 -dc glibmm-2.18.1.tar.bz2 | { tar x -C tmp2; tar x -C tmp2; } diff -urN tmp{1,2} | diffstat > Pick something other than glibmm- there is something definitely wrong w/ that
> tarball. If in doubt-
>
> mkdir tmp{1,2}
> tar jxf glibmm-2.18.1.tar.bz2 -C tmp1
> bzip2 -dc glibmm-2.18.1.tar.bz2 | { tar x -C tmp2; tar x -C tmp2; }
> diff -urN tmp{1,2} | diffstat
Actually libgnome-keyring does the same thing. Whoever the upstream monkey is, they did something rather unfriendly.
The thing to note, tar-1.22 will unpack the file in full- tar-1.23 stops whereve it encounters the tar EOS (first I've really heard of tar having one).
So... going by the other tar implementation behaviours, I'm thinking tar-1.23 is actually compliant while tar 1.22 just sucks.
As for the assert hack portage leveled, it actually would cover up this issue and result in the tarball being only partially unpacked- this obviously isn't desirable.
this isn't pms as much gnome-mm... the tarball's are whacked (only gnutar 1.22 seems to unpack them correctly- 1.23 doesn't anymore), not much to be done about it in PMS. (In reply to comment #28) > this isn't pms as much gnome-mm... the tarball's are whacked (only gnutar 1.22 > seems to unpack them correctly- 1.23 doesn't anymore), not much to be done > about it in PMS. Indeed... Anyone already told upstream that their release stuff is borked? And clearly not the first time this happened, so someone there is using some junky tool to produce their release tarballs; evolution-data-server-2.28.3.1.tar.bz2 is also broken. what's gnome assigned for ??? (In reply to comment #30) > what's gnome assigned for ??? So you can talk to upstream and get them to fix their tarballs. Hello I have reported this upstream but they think nothing special are doing. Would be nice if anyone with more knowledge than me about this issue could comment on upstream bug where think the problem could be https://bugzilla.gnome.org/show_bug.cgi?id=613808 Thanks (In reply to comment #32) > I have reported this upstream but they think nothing special are doing. Would > be nice if anyone with more knowledge than me about this issue could comment on > upstream bug where think the problem could be > https://bugzilla.gnome.org/show_bug.cgi?id=613808 Well, the problem is that their release tarballs are borked. They can grab vanilla upstream tar and verify. And they can use the same vanilla upstream tar to produce proper tarballs instead of this broken stuff. This has borked for like 1 1/2 years and only worked on Gentoo by reverting tar upstream changes (cf. Bug 252680 Comment #4 - see tar-1.21-revert-pipe.patch in ${PORTDIR}/app-arch/tar/files) If you look at the bunzip2ed glibmm-2.18.1.tar you'll see that it's two concatenated tarballs of lengths 28303360 and 28395520 bytes, respectively. The first part contains 3880 files, the second part 3892 files (a superset of the first part!). So something is definitely wrong with this tarball. tar-1.22 on the whole glibmm-2.18.1.tar unpacks only the first part (3880 files) and ignores the rest. note that the current SIGPIPE behavior is what upstream tar says is currently expected practice: http://lists.gnu.org/archive/html/bug-tar/2008-12/msg00037.html so unless you have concrete examples i can take upstream, then the PM should be ignoring SIGPIPE from the decompresser when used with tar for now, i'll re-add the revert-pipe patch, but once PM's are released with this, i'll drop the patch in favor of blockers note that the GNOME peeps should still fix their stuff ... shipping tarballs that are 2x the size of what they should be is still wrong CVE-2010-0624 is still not patched (at least not according to changelog), can we get a fixed version into the tree? this isnt the bug to ask for stabilization Created attachment 244307 [details, diff]
handle SIGPIPE from decompressor
In order to do stricter error checking, with this patch it will die if pipe writers that exited abnormally were not killed by SIGPIPE.
the usage of bitwise operations makes it look dicey. instead, i would go with what bash has documented:
The return value of a simple command is its exit status, or 128+n if the command is terminated by signal n.
unfortunately, while POSIX is clear that SIGKILL must be "9" (and similar mappings for other "core" signal types), i dont believe SIGPIPE is required to be "13". although from what i can tell, all linux ports thus far do define it as 13 (and the `kill` man page defines it as a static binding of 13<->SIGPIPE).
back to the point ... i would simply make the check:
# Allow SIGPIPE through (128 + 13)
[[ $x -ne 0 && $x -ne 141 ]] && die "$@"
also, i dont think you need to bother storing the value in pipestatus:
for x in ${PIPESTATUS[*]} ; do
(In reply to comment #39) > back to the point ... i would simply make the check: > # Allow SIGPIPE through (128 + 13) > [[ $x -ne 0 && $x -ne 141 ]] && die "$@" That looks good. > also, i dont think you need to bother storing the value in pipestatus: > for x in ${PIPESTATUS[*]} ; do The thing is, if we want x to be a local variable then we have no choice but to assign ${PIPESTATUS[*]} to a local in the same declaration, otherwise ${PIPESTATUS[*]} from the caller is lost (replaced by the status of the local declaration). Test cases: $ foo () { local x ; echo ${PIPESTATUS[@]}; } $ true | true $ foo 0 $ foo () { echo ${PIPESTATUS[@]}; } $ true | true $ foo 0 0 Created attachment 244315 [details, diff]
handle SIGPIPE from decompressor
This is the same as the previous patch but it uses 128+N as documented in the bash manual.
indeed ... i missed that subtlety only other feedback would be about the naming. "assert_sigpipe_ok" might be better since it isnt really specific to tar ? (In reply to comment #42) > only other feedback would be about the naming. "assert_sigpipe_ok" might be > better since it isnt really specific to tar ? Good idea. It's in git now: http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=0ab6f83070c9a32a24454053319c99559c2654ca (In reply to comment #43) > http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=0ab6f83070c9a32a24454053319c99559c2654ca This is in portage-2.2_rc69, and will also be in the portage-2.1.9 release that is coming soon. As can be read in upstream report, this should be fixed now and, if any package still fails, a bug for that one should be opened |
* USE: amd64 elibc_glibc kernel_linux userland_GNU >>> Unpacking source... >>> Unpacking libgnome-keyring-2.29.4.tar.bz2 to /var/tmp/portage/gnome-base/libgnome-keyring-2.29.4/work * ERROR: gnome-base/libgnome-keyring-2.29.4 failed: * failure unpacking libgnome-keyring-2.29.4.tar.bz2 * * Call stack: * ebuild.sh, line 48: Called src_unpack * environment, line 1583: Called gnome2_src_unpack * environment, line 1340: Called unpack 'libgnome-keyring-2.29.4.tar.bz2' * ebuild.sh, line 385: Called _unpack_tar 'bzip2' * ebuild.sh, line 360: Called assert 'failure unpacking libgnome-keyring-2.29.4.tar.bz2' * isolated-functions.sh, line 14: Called die * The specific snippet of code: * [[ $x -eq 0 ]] || die "$@" Same thing happens with evolution-data-server. For the time being I'm going to mask tar-1.23