| Message ID | 87d09927hw.fsf@GlaDOS.home |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#40115] download: Use correct system and guile in 'url-fetch/tarbomb' and 'url-fetch/zipbomb'. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | fail | View Laminar job |
Hey Guix, Here's some additional information. Diego Nicola Barbato <dnbarbato@posteo.de> writes: > The attached patch fixes a bug where e.g. > > guix build -s i686-linux ffmpeg > > builds a different derivation on i686-linux than on x86_64-linux. This > doesn't just affect ffmpeg but a whole class of packages which use or > depend on a package that uses 'url-fetch/tarbomb' or 'url-fetch/zipbomb' > as the origin method of its source. That's around 334 packages, among > them diffoscope, enlightenment, gnome, ungoogled-chromium, and wine. The number (348 for commit 151f3d4) and full list of affected packages can be computed by loading the attached script [0] into `guix repl' and running `(show-affected-packages)'. > The problem is fixed by explicitly passing the correct #:system and > #:guile-for-build to 'gexp->derivation' (as is done in other origin > methods such as 'git-fetch' or 'hg-fetch'). > > This shouldn't trigger any rebuils as it only affects the behaviour of > `guix build -s $system $package' if $system differs from the system type > of Guix itself. A closer look at some derivations and outputs suggests that this patch will actually trigger rebuilds for all affected packages on all systems except x86_64 because the build farm currently builds the wrong derivations as can be seen for e.g. QEMU by comparing the build on Cuirass https://ci.guix.gnu.org/build/2442001/details with the derivations computed by guix build -s i686-linux --no-grafts -d qemu on i686-linux and x86_64-linux (commit 151f3d4) respectively: Cuirass: /gnu/store/wc2k8h4iahbnfvl35220hvdx6mc70v7l-qemu-4.2.0.drv /gnu/store/fjg87f21qdzi7h5pqsxpd6rlf9mcy58h-qemu-4.2.0 <~ i686-linux: /gnu/store/019ccjdh1nxfkpjyzwmirvif1ra9v3lh-qemu-4.2.0.drv /gnu/store/8a0cg5ip9967y54gkwskfxmiwwk9mf1b-qemu-4.2.0 x86_64-linux: /gnu/store/iajzrw7lahcyhgyr7anmcjxa33607nqh-qemu-4.2.0.drv /gnu/store/fjg87f21qdzi7h5pqsxpd6rlf9mcy58h-qemu-4.2.0 <~ Consequently no substitutes are available for the affected packages on systems other than x86_64-linux as witnessed by the different number of available substitutes reported by guix weather -s i686-linux -m tarbomb-zipbomb-manifest-small.scm on i686-linux --8<---------------cut here---------------start------------->8--- computing 37 package derivations for i686-linux... looking for 37 store items on https://ci.guix.gnu.org... https://ci.guix.gnu.org 18.9% substitutes available (7 out of 37) at least 2.3 MiB of nars (compressed) 5.1 MiB on disk (uncompressed) 0.001 seconds per request (0.0 seconds in total) 1028.5 requests per second 'https://ci.guix.gnu.org/api/queue?nr=1000' returned 504 ("Gateway Time-out") --8<---------------cut here---------------end--------------->8--- and on x86_64-linux --8<---------------cut here---------------start------------->8--- computing 37 package derivations for i686-linux... looking for 37 store items on https://ci.guix.gnu.org... https://ci.guix.gnu.org 81.1% substitutes available (30 out of 37) at least 165.9 MiB of nars (compressed) 423.3 MiB on disk (uncompressed) 0.001 seconds per request (0.1 seconds in total) 703.3 requests per second 'https://ci.guix.gnu.org/api/queue?nr=1000' returned 504 ("Gateway Time-out") --8<---------------cut here---------------end--------------->8--- I have attached manifest files for the packages directly using `url-fetch/tarbomb' or `url-fetch/zipbomb' [1] and for all affected packages [2] (they use the aforementioned script). I think this patch can go on master even though it triggers more than 300 rebuilds, since there are currently no substitutes available for the affected packages anyway. Regards, Diego PS I hope I got all the terminology (e.g. computing vs. building a derivation) right. [0]: [1]: [2]:
Hi Diego, Diego Nicola Barbato <dnbarbato@posteo.de> skribis: >>From 85594ce40c98ac5763b8295e2358567c6920188e Mon Sep 17 00:00:00 2001 > From: Diego Nicola Barbato <dnbarbato@posteo.de> > Date: Mon, 16 Mar 2020 18:43:20 +0100 > Subject: [PATCH] download: Use correct system and guile in 'url-fetch/tarbomb' > and 'url-fetch/zipbomb'. > > Previously the result of `guix build -s $system $package' would depend on the > system Guix was built for if $package or one of its dependencies used > 'url-fetch/tarbomb' or 'url-fetch/zipbomb' as the origin method of its > source (e.g. `guix build -s i686-linux ffmpeg' on i686-linux would build a > different derivation than on x86_64-linux). > > This patch fixes this by explicitly passing the correct system and guile to > 'gexp->derivation'. > > * guix/download.scm (url-fetch/tarbomb): Pass #:system system and > #:guile-for-build guile to 'gexp->derivation', where guile is the derivation > of guile for system. > (url-fetch/zipbomb): Likewise. Good catch, pushed as c1d81df93d4b67671fc4a8e0a80c0f02c5821663! >> builds a different derivation on i686-linux than on x86_64-linux. This >> doesn't just affect ffmpeg but a whole class of packages which use or >> depend on a package that uses 'url-fetch/tarbomb' or 'url-fetch/zipbomb' >> as the origin method of its source. That's around 334 packages, among >> them diffoscope, enlightenment, gnome, ungoogled-chromium, and wine. > > The number (348 for commit 151f3d4) and full list of affected packages > can be computed by loading the attached script [0] into `guix repl' and > running `(show-affected-packages)'. Terrible that ci. has been building useless substitutes for these packages and that users of non-x86_64 platforms were not getting substitutes. Thanks a lot for the patch and detailed analysis! Ludo’.
From 85594ce40c98ac5763b8295e2358567c6920188e Mon Sep 17 00:00:00 2001 From: Diego Nicola Barbato <dnbarbato@posteo.de> Date: Mon, 16 Mar 2020 18:43:20 +0100 Subject: [PATCH] download: Use correct system and guile in 'url-fetch/tarbomb' and 'url-fetch/zipbomb'. Previously the result of `guix build -s $system $package' would depend on the system Guix was built for if $package or one of its dependencies used 'url-fetch/tarbomb' or 'url-fetch/zipbomb' as the origin method of its source (e.g. `guix build -s i686-linux ffmpeg' on i686-linux would build a different derivation than on x86_64-linux). This patch fixes this by explicitly passing the correct system and guile to 'gexp->derivation'. * guix/download.scm (url-fetch/tarbomb): Pass #:system system and #:guile-for-build guile to 'gexp->derivation', where guile is the derivation of guile for system. (url-fetch/zipbomb): Likewise. --- guix/download.scm | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/guix/download.scm b/guix/download.scm index 91a2b4ce5f..c3dc5a208c 100644 --- a/guix/download.scm +++ b/guix/download.scm @@ -531,7 +531,8 @@ own. This helper makes it easier to deal with \"tar bombs\"." (string-append "tarbomb-" (or name file-name)) #:system system - #:guile guile))) + #:guile guile)) + (guile (package->derivation guile system))) ;; Take the tar bomb, and simply unpack it as a directory. ;; Use ungrafted tar/gzip so that the resulting tarball doesn't depend on ;; whether grafts are enabled. @@ -544,6 +545,8 @@ own. This helper makes it easier to deal with \"tar bombs\"." (chdir #$output) (invoke (string-append #$tar "/bin/tar") "xf" #$drv))) + #:system system + #:guile-for-build guile #:graft? #f #:local-build? #t))) @@ -566,7 +569,8 @@ own. This helper makes it easier to deal with \"zip bombs\"." (string-append "zipbomb-" (or name file-name)) #:system system - #:guile guile))) + #:guile guile)) + (guile (package->derivation guile system))) ;; Take the zip bomb, and simply unpack it as a directory. ;; Use ungrafted unzip so that the resulting tarball doesn't depend on ;; whether grafts are enabled. @@ -578,6 +582,8 @@ own. This helper makes it easier to deal with \"zip bombs\"." (chdir #$output) (invoke (string-append #$unzip "/bin/unzip") #$drv))) + #:system system + #:guile-for-build guile #:graft? #f #:local-build? #t))) -- 2.25.1