* [PATCH 0/2] Perl: Enable threading support.
@ 2016-09-20 4:56 Ben Woodcroft
2016-09-20 4:56 ` [PATCH 1/2] gnu: perl: Split configure phase Ben Woodcroft
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Ben Woodcroft @ 2016-09-20 4:56 UTC (permalink / raw)
To: guix-devel
Hi,
I found that our Perl was giving "Error: This Perl not built to support
threads" when trying to use threads. I added '-Dusethreads' to the configure
phase, but had to copy across the old configure phase to the inheriting
'perl-boot0' where we cannot use threads as pthreads is apparently
unavailable. Perhaps there is a better way than simply copying the configure
phase code, though I did add comments pointing out the duplication.
I'm a little lost as to where we are in the cycle. If the patches are OK do I
push this 'rebuild the world' change to 'core-updates', or make a new
'core-updates-next'?
Thanks in advance. It might help to view the second patch with 'git diff -w'.
ben
[PATCH 1/2] gnu: perl: Split configure phase.
[PATCH 2/2] gnu: perl: Enable threading support.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] gnu: perl: Split configure phase.
2016-09-20 4:56 [PATCH 0/2] Perl: Enable threading support Ben Woodcroft
@ 2016-09-20 4:56 ` Ben Woodcroft
2016-09-24 5:02 ` Ludovic Courtès
2016-09-20 4:56 ` [PATCH 2/2] gnu: perl: Enable threading support Ben Woodcroft
2016-09-20 20:58 ` [PATCH 0/2] Perl: Enable threading support Eric Bavier
2 siblings, 1 reply; 15+ messages in thread
From: Ben Woodcroft @ 2016-09-20 4:56 UTC (permalink / raw)
To: guix-devel
* gnu/packages/perl.scm (perl)[arguments]: Split 'configure' phase into
'setup-configure' and 'configure' phases.
---
gnu/packages/perl.scm | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
index 0a26e51..f0c4e36 100644
--- a/gnu/packages/perl.scm
+++ b/gnu/packages/perl.scm
@@ -62,22 +62,24 @@
'(#:tests? #f
#:phases
(modify-phases %standard-phases
+ (add-before 'configure 'setup-configure
+ (lambda _
+ ;; Use the right path for `pwd'.
+ (substitute* "dist/PathTools/Cwd.pm"
+ (("/bin/pwd")
+ (which "pwd")))
+
+ ;; Build in GNU89 mode to tolerate C++-style comment in libc's
+ ;; <bits/string3.h>.
+ (substitute* "cflags.SH"
+ (("-std=c89")
+ "-std=gnu89"))
+ #t))
(replace
'configure
(lambda* (#:key inputs outputs #:allow-other-keys)
(let ((out (assoc-ref outputs "out"))
(libc (assoc-ref inputs "libc")))
- ;; Use the right path for `pwd'.
- (substitute* "dist/PathTools/Cwd.pm"
- (("/bin/pwd")
- (which "pwd")))
-
- ;; Build in GNU89 mode to tolerate C++-style comment in libc's
- ;; <bits/string3.h>.
- (substitute* "cflags.SH"
- (("-std=c89")
- "-std=gnu89"))
-
(zero?
(system* "./Configure"
(string-append "-Dprefix=" out)
--
2.10.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] gnu: perl: Enable threading support.
2016-09-20 4:56 [PATCH 0/2] Perl: Enable threading support Ben Woodcroft
2016-09-20 4:56 ` [PATCH 1/2] gnu: perl: Split configure phase Ben Woodcroft
@ 2016-09-20 4:56 ` Ben Woodcroft
2016-09-24 5:05 ` Ludovic Courtès
2016-09-20 20:58 ` [PATCH 0/2] Perl: Enable threading support Eric Bavier
2 siblings, 1 reply; 15+ messages in thread
From: Ben Woodcroft @ 2016-09-20 4:56 UTC (permalink / raw)
To: guix-devel
* gnu/packages/perl.scm (perl)[arguments]: Enable threading support.
* gnu/packages/commencement.scm (perl-boot0): Do not inherit 'configure'
phase from perl.
---
gnu/packages/commencement.scm | 57 +++++++++++++++++++++++++++++--------------
gnu/packages/perl.scm | 4 +++
2 files changed, 43 insertions(+), 18 deletions(-)
diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 8f1ecf8..60822a9 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2014 Andreas Enge <andreas@enge.fr>
;;; Copyright © 2012 Nikita Karetnikov <nikita@karetnikov.org>
;;; Copyright © 2014, 2015 Mark H Weaver <mhw@netris.org>
+;;; Copyright © 2016 Ben Woodcroft <donttrustben@gmail.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -269,24 +270,44 @@
(package-native-inputs gcc))))))
(define perl-boot0
- (let ((perl (package
- (inherit perl)
- (name "perl-boot0")
- (replacement #f)
- (arguments
- ;; At the very least, this must not depend on GCC & co.
- (let ((args `(#:disallowed-references
- ,(list %bootstrap-binutils))))
- (substitute-keyword-arguments (package-arguments perl)
- ((#:phases phases)
- `(modify-phases ,phases
- ;; Pthread support is missing in the bootstrap compiler
- ;; (broken spec file), so disable it.
- (add-before 'configure 'disable-pthreads
- (lambda _
- (substitute* "Configure"
- (("^libswanted=(.*)pthread" _ before)
- (string-append "libswanted=" before)))))))))))))
+ (let ((perl
+ (package
+ (inherit perl)
+ (name "perl-boot0")
+ (replacement #f)
+ (arguments
+ ;; At the very least, this must not depend on GCC & co.
+ (let ((args `(#:disallowed-references
+ ,(list %bootstrap-binutils))))
+ (substitute-keyword-arguments (package-arguments perl)
+ ((#:phases phases)
+ `(modify-phases ,phases
+ ;; Pthread support is missing in the bootstrap compiler
+ ;; (broken spec file), so disable it.
+ (add-before 'configure 'disable-pthreads
+ (lambda _
+ (substitute* "Configure"
+ (("^libswanted=(.*)pthread" _ before)
+ (string-append "libswanted=" before)))))
+ ;; This phase is largely shared with 'perl' but we
+ ;; configure without threading support.
+ (replace 'configure
+ (lambda* (#:key inputs outputs #:allow-other-keys)
+ (let ((out (assoc-ref outputs "out"))
+ (libc (assoc-ref inputs "libc")))
+ (zero?
+ (system*
+ "./Configure"
+ (string-append "-Dprefix=" out)
+ (string-append "-Dman1dir=" out "/share/man/man1")
+ (string-append "-Dman3dir=" out "/share/man/man3")
+ "-de" "-Dcc=gcc"
+ "-Uinstallusrbinperl"
+ "-Dinstallstyle=lib/perl5"
+ "-Duseshrplib"
+ (string-append "-Dlocincpth=" libc "/include")
+ (string-append "-Dloclibpth=" libc
+ "/lib"))))))))))))))
(package-with-bootstrap-guile
(package-with-explicit-inputs perl
%boot0-inputs
diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
index f0c4e36..273e4d0 100644
--- a/gnu/packages/perl.scm
+++ b/gnu/packages/perl.scm
@@ -77,6 +77,9 @@
#t))
(replace
'configure
+ ;; This configure phase is largely shared with 'perl-boot0', so if
+ ;; changes are made in this phase they may also be applicable
+ ;; there.
(lambda* (#:key inputs outputs #:allow-other-keys)
(let ((out (assoc-ref outputs "out"))
(libc (assoc-ref inputs "libc")))
@@ -89,6 +92,7 @@
"-Uinstallusrbinperl"
"-Dinstallstyle=lib/perl5"
"-Duseshrplib"
+ "-Dusethreads"
(string-append "-Dlocincpth=" libc "/include")
(string-append "-Dloclibpth=" libc "/lib"))))))
--
2.10.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] Perl: Enable threading support.
2016-09-20 4:56 [PATCH 0/2] Perl: Enable threading support Ben Woodcroft
2016-09-20 4:56 ` [PATCH 1/2] gnu: perl: Split configure phase Ben Woodcroft
2016-09-20 4:56 ` [PATCH 2/2] gnu: perl: Enable threading support Ben Woodcroft
@ 2016-09-20 20:58 ` Eric Bavier
2 siblings, 0 replies; 15+ messages in thread
From: Eric Bavier @ 2016-09-20 20:58 UTC (permalink / raw)
To: Ben Woodcroft; +Cc: guix-devel
On Tue, 20 Sep 2016 14:56:05 +1000
Ben Woodcroft <donttrustben@gmail.com> wrote:
> Hi,
>
> I found that our Perl was giving "Error: This Perl not built to support
> threads" when trying to use threads. I added '-Dusethreads' to the configure
> phase, but had to copy across the old configure phase to the inheriting
> 'perl-boot0' where we cannot use threads as pthreads is apparently
> unavailable. Perhaps there is a better way than simply copying the configure
> phase code, though I did add comments pointing out the duplication.
>
> I'm a little lost as to where we are in the cycle. If the patches are OK do I
> push this 'rebuild the world' change to 'core-updates', or make a new
> 'core-updates-next'?
>
> Thanks in advance. It might help to view the second patch with 'git diff -w'.
> ben
>
> [PATCH 1/2] gnu: perl: Split configure phase.
> [PATCH 2/2] gnu: perl: Enable threading support.
>
Could you instead have perl's configure phase honor a #:threads?
keyword, and have perl-boot0 put "#:threads? #f" in its arguments?
`~Eric
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] gnu: perl: Split configure phase.
2016-09-20 4:56 ` [PATCH 1/2] gnu: perl: Split configure phase Ben Woodcroft
@ 2016-09-24 5:02 ` Ludovic Courtès
0 siblings, 0 replies; 15+ messages in thread
From: Ludovic Courtès @ 2016-09-24 5:02 UTC (permalink / raw)
To: Ben Woodcroft; +Cc: guix-devel
Ben Woodcroft <donttrustben@gmail.com> skribis:
> * gnu/packages/perl.scm (perl)[arguments]: Split 'configure' phase into
> 'setup-configure' and 'configure' phases.
Good, you can already push this one to core-updates.
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gnu: perl: Enable threading support.
2016-09-20 4:56 ` [PATCH 2/2] gnu: perl: Enable threading support Ben Woodcroft
@ 2016-09-24 5:05 ` Ludovic Courtès
2016-09-26 10:03 ` Ben Woodcroft
0 siblings, 1 reply; 15+ messages in thread
From: Ludovic Courtès @ 2016-09-24 5:05 UTC (permalink / raw)
To: Ben Woodcroft; +Cc: guix-devel
Ben Woodcroft <donttrustben@gmail.com> skribis:
> * gnu/packages/perl.scm (perl)[arguments]: Enable threading support.
> * gnu/packages/commencement.scm (perl-boot0): Do not inherit 'configure'
> phase from perl.
[...]
> "-Uinstallusrbinperl"
> "-Dinstallstyle=lib/perl5"
> "-Duseshrplib"
> + "-Dusethreads"
Is -Dusethreads really needed? I thought the default behavior was to
build pthread support if ./Configure detects it. That would greatly
simplify things.
If not, a variant of what Eric suggests would be to honor
#:configure-flags in this phase, such that all you need is to provide
different #:configure-flags in perl-boot0.
HTH!
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gnu: perl: Enable threading support.
2016-09-24 5:05 ` Ludovic Courtès
@ 2016-09-26 10:03 ` Ben Woodcroft
2016-10-01 13:22 ` Ludovic Courtès
0 siblings, 1 reply; 15+ messages in thread
From: Ben Woodcroft @ 2016-09-26 10:03 UTC (permalink / raw)
To: Ludovic Courtès, Ben Woodcroft; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]
On 24/09/16 15:05, Ludovic Courtès wrote:
> Ben Woodcroft <donttrustben@gmail.com> skribis:
>
>> * gnu/packages/perl.scm (perl)[arguments]: Enable threading support.
>> * gnu/packages/commencement.scm (perl-boot0): Do not inherit 'configure'
>> phase from perl.
> [...]
>
>> "-Uinstallusrbinperl"
>> "-Dinstallstyle=lib/perl5"
>> "-Duseshrplib"
>> + "-Dusethreads"
> Is -Dusethreads really needed? I thought the default behavior was to
> build pthread support if ./Configure detects it. That would greatly
> simplify things.
Afraid so. On master:
$ ./pre-inst-env guix environment -C --ad-hoc perl -- perl -e 'use threads'
This Perl not built to support threads
Compilation failed in require at -e line 1.
BEGIN failed--compilation aborted at -e line 1.
> If not, a variant of what Eric suggests would be to honor
> #:configure-flags in this phase, such that all you need is to provide
> different #:configure-flags in perl-boot0.
I like this approach as it is it more general. Attached a 2-in-1 patch
to implement it.
> HTH!
Indeed, good idea thanks.
ben
[-- Attachment #2: perl-threading20160925.patch --]
[-- Type: text/x-patch, Size: 5917 bytes --]
From c61c799da21f349c739f9d094d348ae429a91ffc Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Sat, 24 Sep 2016 22:44:55 +1000
Subject: [PATCH 1/2] gnu: perl: Use configure-flags.
* gnu/packages/perl.scm (perl)[arguments]: Use configure-flags.
---
gnu/packages/perl.scm | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
index f0c4e36..aea05dd 100644
--- a/gnu/packages/perl.scm
+++ b/gnu/packages/perl.scm
@@ -60,6 +60,19 @@
(build-system gnu-build-system)
(arguments
'(#:tests? #f
+ #:configure-flags
+ (let ((out (assoc-ref %outputs "out"))
+ (libc (assoc-ref %build-inputs "libc")))
+ (list
+ (string-append "-Dprefix=" out)
+ (string-append "-Dman1dir=" out "/share/man/man1")
+ (string-append "-Dman3dir=" out "/share/man/man3")
+ "-de" "-Dcc=gcc"
+ "-Uinstallusrbinperl"
+ "-Dinstallstyle=lib/perl5"
+ "-Duseshrplib"
+ (string-append "-Dlocincpth=" libc "/include")
+ (string-append "-Dloclibpth=" libc "/lib")))
#:phases
(modify-phases %standard-phases
(add-before 'configure 'setup-configure
@@ -77,21 +90,9 @@
#t))
(replace
'configure
- (lambda* (#:key inputs outputs #:allow-other-keys)
- (let ((out (assoc-ref outputs "out"))
- (libc (assoc-ref inputs "libc")))
- (zero?
- (system* "./Configure"
- (string-append "-Dprefix=" out)
- (string-append "-Dman1dir=" out "/share/man/man1")
- (string-append "-Dman3dir=" out "/share/man/man3")
- "-de" "-Dcc=gcc"
- "-Uinstallusrbinperl"
- "-Dinstallstyle=lib/perl5"
- "-Duseshrplib"
- (string-append "-Dlocincpth=" libc "/include")
- (string-append "-Dloclibpth=" libc "/lib"))))))
-
+ (lambda* (#:key configure-flags #:allow-other-keys)
+ (zero? (apply system* (append (list "./Configure")
+ configure-flags)))))
(add-before
'strip 'make-shared-objects-writable
(lambda* (#:key outputs #:allow-other-keys)
--
2.10.0
From d382e48d801406897c27b045ab1feb20cfec6db4 Mon Sep 17 00:00:00 2001
From: Ben Woodcroft <donttrustben@gmail.com>
Date: Sat, 24 Sep 2016 23:22:54 +1000
Subject: [PATCH 2/2] gnu: perl: Enable threading support.
* gnu/packages/perl.scm (perl)[arguments]: Configure with '-Dusethreads'.
* gnu/packages/commencement.scm (perl-boot0)[arguments]: Omit inherited
'-Dusethreads' flag during configure.
---
gnu/packages/commencement.scm | 32 ++++++++++++++++++++------------
gnu/packages/perl.scm | 3 ++-
2 files changed, 22 insertions(+), 13 deletions(-)
diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 8f1ecf8..61df290 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -275,18 +275,26 @@
(replacement #f)
(arguments
;; At the very least, this must not depend on GCC & co.
- (let ((args `(#:disallowed-references
- ,(list %bootstrap-binutils))))
- (substitute-keyword-arguments (package-arguments perl)
- ((#:phases phases)
- `(modify-phases ,phases
- ;; Pthread support is missing in the bootstrap compiler
- ;; (broken spec file), so disable it.
- (add-before 'configure 'disable-pthreads
- (lambda _
- (substitute* "Configure"
- (("^libswanted=(.*)pthread" _ before)
- (string-append "libswanted=" before)))))))))))))
+ (let
+ ((args `(#:disallowed-references
+ ,(list %bootstrap-binutils)))
+ (sub1
+ (substitute-keyword-arguments (package-arguments perl)
+ ((#:phases phases)
+ `(modify-phases ,phases
+ ;; Pthread support is missing in the bootstrap
+ ;; compiler (broken spec file), so disable it.
+ (add-before 'configure 'disable-pthreads
+ (lambda _
+ (substitute* "Configure"
+ (("^libswanted=(.*)pthread" _ before)
+ (string-append "libswanted="
+ before))))))))))
+ (substitute-keyword-arguments sub1
+ ;; Do not configure with -Dusethreads since pthread
+ ;; support is missing.
+ ((#:configure-flags configure-flags)
+ `(delete "-Dusethreads" ,configure-flags))))))))
(package-with-bootstrap-guile
(package-with-explicit-inputs perl
%boot0-inputs
diff --git a/gnu/packages/perl.scm b/gnu/packages/perl.scm
index aea05dd..4e1e7e1 100644
--- a/gnu/packages/perl.scm
+++ b/gnu/packages/perl.scm
@@ -72,7 +72,8 @@
"-Dinstallstyle=lib/perl5"
"-Duseshrplib"
(string-append "-Dlocincpth=" libc "/include")
- (string-append "-Dloclibpth=" libc "/lib")))
+ (string-append "-Dloclibpth=" libc "/lib")
+ "-Dusethreads"))
#:phases
(modify-phases %standard-phases
(add-before 'configure 'setup-configure
--
2.10.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] gnu: perl: Enable threading support.
2016-09-26 10:03 ` Ben Woodcroft
@ 2016-10-01 13:22 ` Ludovic Courtès
2016-10-01 16:40 ` Core-updates timeline (was: Re: [PATCH 2/2] gnu: perl: Enable threading support.) Leo Famulari
0 siblings, 1 reply; 15+ messages in thread
From: Ludovic Courtès @ 2016-10-01 13:22 UTC (permalink / raw)
To: Ben Woodcroft; +Cc: guix-devel
Hi Ben,
Ben Woodcroft <b.woodcroft@uq.edu.au> skribis:
> From c61c799da21f349c739f9d094d348ae429a91ffc Mon Sep 17 00:00:00 2001
> From: Ben Woodcroft <donttrustben@gmail.com>
> Date: Sat, 24 Sep 2016 22:44:55 +1000
> Subject: [PATCH 1/2] gnu: perl: Use configure-flags.
>
> * gnu/packages/perl.scm (perl)[arguments]: Use configure-flags.
[...]
> From d382e48d801406897c27b045ab1feb20cfec6db4 Mon Sep 17 00:00:00 2001
> From: Ben Woodcroft <donttrustben@gmail.com>
> Date: Sat, 24 Sep 2016 23:22:54 +1000
> Subject: [PATCH 2/2] gnu: perl: Enable threading support.
>
> * gnu/packages/perl.scm (perl)[arguments]: Configure with '-Dusethreads'.
> * gnu/packages/commencement.scm (perl-boot0)[arguments]: Omit inherited
> '-Dusethreads' flag during configure.
I pushed simplified versions of these two patches as
56ee1d2015e9b2c55d34f19c70b06eefe8a20c76 and
156c0810e936413ac554e2883343b3b40695cfdc.
I think this was the last non-bug-fix change for this core-updates
cycle. :-)
Thanks!
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Core-updates timeline (was: Re: [PATCH 2/2] gnu: perl: Enable threading support.)
2016-10-01 13:22 ` Ludovic Courtès
@ 2016-10-01 16:40 ` Leo Famulari
2016-10-02 13:38 ` Core-updates timeline Ludovic Courtès
0 siblings, 1 reply; 15+ messages in thread
From: Leo Famulari @ 2016-10-01 16:40 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
On Sat, Oct 01, 2016 at 03:22:36PM +0200, Ludovic Courtès wrote:
> I pushed simplified versions of these two patches as
> 56ee1d2015e9b2c55d34f19c70b06eefe8a20c76 and
> 156c0810e936413ac554e2883343b3b40695cfdc.
>
> I think this was the last non-bug-fix change for this core-updates
> cycle. :-)
Cool :)
I've been waiting for libarchive 3.2.2 [0] so that I can graft it on
master and ungraft it on core-updates. But, I can try cherry-picking the
most important changes today. When do you hope to freeze core-updates?
Do you want me to try cherry-picking?
https://github.com/libarchive/libarchive/milestone/4
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Core-updates timeline
2016-10-01 16:40 ` Core-updates timeline (was: Re: [PATCH 2/2] gnu: perl: Enable threading support.) Leo Famulari
@ 2016-10-02 13:38 ` Ludovic Courtès
2016-10-02 18:50 ` Leo Famulari
2016-10-07 20:16 ` Core-updates timeline Ludovic Courtès
0 siblings, 2 replies; 15+ messages in thread
From: Ludovic Courtès @ 2016-10-02 13:38 UTC (permalink / raw)
To: Leo Famulari; +Cc: guix-devel
Hi!
Leo Famulari <leo@famulari.name> skribis:
> On Sat, Oct 01, 2016 at 03:22:36PM +0200, Ludovic Courtès wrote:
>> I pushed simplified versions of these two patches as
>> 56ee1d2015e9b2c55d34f19c70b06eefe8a20c76 and
>> 156c0810e936413ac554e2883343b3b40695cfdc.
>>
>> I think this was the last non-bug-fix change for this core-updates
>> cycle. :-)
>
> Cool :)
>
> I've been waiting for libarchive 3.2.2 [0] so that I can graft it on
> master and ungraft it on core-updates. But, I can try cherry-picking the
> most important changes today. When do you hope to freeze core-updates?
> Do you want me to try cherry-picking?
There’s was another Bash-related issue that I just fixed, and Hydra is
now rebuilding the “core” package subset. If everything is fine when
it’s done, which could be tomorrow, we could freeze.
We could wait an additional day for libarchive if it’s more convenient,
but maybe not longer than that.
What do you think would be the most convenient approach?
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Core-updates timeline
2016-10-02 13:38 ` Core-updates timeline Ludovic Courtès
@ 2016-10-02 18:50 ` Leo Famulari
2016-10-02 20:14 ` libarchive security fixes (was Re: Core-updates timeline) Leo Famulari
2016-10-07 20:16 ` Core-updates timeline Ludovic Courtès
1 sibling, 1 reply; 15+ messages in thread
From: Leo Famulari @ 2016-10-02 18:50 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
On Sun, Oct 02, 2016 at 03:38:58PM +0200, Ludovic Courtès wrote:
> Hi!
>
> Leo Famulari <leo@famulari.name> skribis:
>
> > On Sat, Oct 01, 2016 at 03:22:36PM +0200, Ludovic Courtès wrote:
> >> I pushed simplified versions of these two patches as
> >> 56ee1d2015e9b2c55d34f19c70b06eefe8a20c76 and
> >> 156c0810e936413ac554e2883343b3b40695cfdc.
> >>
> >> I think this was the last non-bug-fix change for this core-updates
> >> cycle. :-)
> >
> > Cool :)
> >
> > I've been waiting for libarchive 3.2.2 [0] so that I can graft it on
> > master and ungraft it on core-updates. But, I can try cherry-picking the
> > most important changes today. When do you hope to freeze core-updates?
> > Do you want me to try cherry-picking?
>
> There’s was another Bash-related issue that I just fixed, and Hydra is
> now rebuilding the “core” package subset. If everything is fine when
> it’s done, which could be tomorrow, we could freeze.
>
> We could wait an additional day for libarchive if it’s more convenient,
> but maybe not longer than that.
>
> What do you think would be the most convenient approach?
I will send a patch that cherry-picks what I think are the most
important bug fixes. I can't guess when libarchive 3.2.2 will be
released.
^ permalink raw reply [flat|nested] 15+ messages in thread
* libarchive security fixes (was Re: Core-updates timeline)
2016-10-02 18:50 ` Leo Famulari
@ 2016-10-02 20:14 ` Leo Famulari
2016-10-03 16:10 ` Ludovic Courtès
0 siblings, 1 reply; 15+ messages in thread
From: Leo Famulari @ 2016-10-02 20:14 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
[-- Attachment #1.1: Type: text/plain, Size: 1060 bytes --]
On Sun, Oct 02, 2016 at 02:50:34PM -0400, Leo Famulari wrote:
> On Sun, Oct 02, 2016 at 03:38:58PM +0200, Ludovic Courtès wrote:
> > We could wait an additional day for libarchive if it’s more convenient,
> > but maybe not longer than that.
> >
> > What do you think would be the most convenient approach?
>
> I will send a patch that cherry-picks what I think are the most
> important bug fixes. I can't guess when libarchive 3.2.2 will be
> released.
I've attached a patch.
It cherry-picks some fixes for some filesystem attacks and two overflows
that can be triggered with "crafted" input. The details are in the patch
files.
I understand if this approach of cherry-picking a handful of commits is
not acceptable. It's hard to judge the full impact of taking only these
changes, some of which a quite significant, without being familiar with
the libarchive code.
That's the reason why I've been waiting for a new upstream release. But
I figured I should at least try to get these bug fixes into the next
release of Guix :)
[-- Attachment #1.2: 0001-gnu-libarchive-Fix-several-security-issues.patch --]
[-- Type: text/plain, Size: 24051 bytes --]
From 042d5a7df4962c3b81fbfefa0027b6f1cf356b5f Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Sun, 2 Oct 2016 15:58:06 -0400
Subject: [PATCH] gnu: libarchive: Fix several security issues.
* gnu/packages/backup.scm (libarchive)[replacement]: New field.
(libarchive/fixed): New variable.
* gnu/packages/patches/libarchive-7zip-heap-overflow.patch,
gnu/packages/patches/libarchive-fix-symlink-check.patch,
gnu/packages/patches/libarchive-fix-filesystem-attacks.patch,
gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch: New files.
* gnu/local.mk (dist_patch_DATA): Add them.
---
gnu/local.mk | 4 +
gnu/packages/backup.scm | 12 +
.../patches/libarchive-7zip-heap-overflow.patch | 77 ++++
.../libarchive-fix-filesystem-attacks.patch | 445 +++++++++++++++++++++
.../libarchive-safe_fprintf-buffer-overflow.patch | 44 ++
5 files changed, 582 insertions(+)
create mode 100644 gnu/packages/patches/libarchive-7zip-heap-overflow.patch
create mode 100644 gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
create mode 100644 gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch
diff --git a/gnu/local.mk b/gnu/local.mk
index 4260a92..02cd680 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -622,6 +622,10 @@ dist_patch_DATA = \
%D%/packages/patches/liba52-link-with-libm.patch \
%D%/packages/patches/liba52-set-soname.patch \
%D%/packages/patches/liba52-use-mtune-not-mcpu.patch \
+ %D%/packages/patches/libarchive-7zip-heap-overflow.patch \
+ %D%/packages/patches/libarchive-fix-symlink-check.patch \
+ %D%/packages/patches/libarchive-fix-filesystem-attacks.patch \
+ %D%/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch \
%D%/packages/patches/libbonobo-activation-test-race.patch \
%D%/packages/patches/libcanberra-sound-theme-freedesktop.patch \
%D%/packages/patches/libcmis-fix-test-onedrive.patch \
diff --git a/gnu/packages/backup.scm b/gnu/packages/backup.scm
index c6f1321..797c06e 100644
--- a/gnu/packages/backup.scm
+++ b/gnu/packages/backup.scm
@@ -172,6 +172,7 @@ backups (called chunks) to allow easy burning to CD/DVD.")
(define-public libarchive
(package
(name "libarchive")
+ (replacement libarchive/fixed)
(version "3.2.1")
(source
(origin
@@ -227,6 +228,17 @@ archive. In particular, note that there is currently no built-in support for
random access nor for in-place modification.")
(license license:bsd-2)))
+(define libarchive/fixed
+ (package
+ (inherit libarchive)
+ (source (origin
+ (inherit (package-source libarchive))
+ (patches (search-patches
+ "libarchive-7zip-heap-overflow.patch"
+ "libarchive-fix-symlink-check.patch"
+ "libarchive-fix-filesystem-attacks.patch"
+ "libarchive-safe_fprintf-buffer-overflow.patch"))))))
+
(define-public rdup
(package
(name "rdup")
diff --git a/gnu/packages/patches/libarchive-7zip-heap-overflow.patch b/gnu/packages/patches/libarchive-7zip-heap-overflow.patch
new file mode 100644
index 0000000..bef628f
--- /dev/null
+++ b/gnu/packages/patches/libarchive-7zip-heap-overflow.patch
@@ -0,0 +1,77 @@
+Fix buffer overflow reading 7Zip files:
+
+https://github.com/libarchive/libarchive/issues/761
+
+Patch copied from upstream repository:
+
+https://github.com/libarchive/libarchive/commit/7f17c791dcfd8c0416e2cd2485b19410e47ef126
+
+From 7f17c791dcfd8c0416e2cd2485b19410e47ef126 Mon Sep 17 00:00:00 2001
+From: Tim Kientzle <kientzle@acm.org>
+Date: Sun, 18 Sep 2016 18:14:58 -0700
+Subject: [PATCH] Issue 761: Heap overflow reading corrupted 7Zip files
+
+The sample file that demonstrated this had multiple 'EmptyStream'
+attributes. The first one ended up being used to calculate
+certain statistics, then was overwritten by the second which
+was incompatible with those statistics.
+
+The fix here is to reject any header with multiple EmptyStream
+attributes. While here, also reject headers with multiple
+EmptyFile, AntiFile, Name, or Attributes markers.
+---
+ libarchive/archive_read_support_format_7zip.c | 10 ++++++++++
+ 1 file changed, 10 insertions(+)
+
+diff --git a/libarchive/archive_read_support_format_7zip.c b/libarchive/archive_read_support_format_7zip.c
+index 1dfe52b..c0a536c 100644
+--- a/libarchive/archive_read_support_format_7zip.c
++++ b/libarchive/archive_read_support_format_7zip.c
+@@ -2431,6 +2431,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+
+ switch (type) {
+ case kEmptyStream:
++ if (h->emptyStreamBools != NULL)
++ return (-1);
+ h->emptyStreamBools = calloc((size_t)zip->numFiles,
+ sizeof(*h->emptyStreamBools));
+ if (h->emptyStreamBools == NULL)
+@@ -2451,6 +2453,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ return (-1);
+ break;
+ }
++ if (h->emptyFileBools != NULL)
++ return (-1);
+ h->emptyFileBools = calloc(empty_streams,
+ sizeof(*h->emptyFileBools));
+ if (h->emptyFileBools == NULL)
+@@ -2465,6 +2469,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ return (-1);
+ break;
+ }
++ if (h->antiBools != NULL)
++ return (-1);
+ h->antiBools = calloc(empty_streams,
+ sizeof(*h->antiBools));
+ if (h->antiBools == NULL)
+@@ -2491,6 +2497,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ if ((ll & 1) || ll < zip->numFiles * 4)
+ return (-1);
+
++ if (zip->entry_names != NULL)
++ return (-1);
+ zip->entry_names = malloc(ll);
+ if (zip->entry_names == NULL)
+ return (-1);
+@@ -2543,6 +2551,8 @@ read_Header(struct archive_read *a, struct _7z_header_info *h,
+ if ((p = header_bytes(a, 2)) == NULL)
+ return (-1);
+ allAreDefined = *p;
++ if (h->attrBools != NULL)
++ return (-1);
+ h->attrBools = calloc((size_t)zip->numFiles,
+ sizeof(*h->attrBools));
+ if (h->attrBools == NULL)
+--
+2.10.0
+
diff --git a/gnu/packages/patches/libarchive-fix-filesystem-attacks.patch b/gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
new file mode 100644
index 0000000..bce63d5
--- /dev/null
+++ b/gnu/packages/patches/libarchive-fix-filesystem-attacks.patch
@@ -0,0 +1,445 @@
+This patch fixes two bugs that allow attackers to overwrite or change
+the permissions of arbitrary files:
+
+https://github.com/libarchive/libarchive/issues/745
+https://github.com/libarchive/libarchive/issues/746
+
+Patch copied from upstream repository:
+
+https://github.com/libarchive/libarchive/commit/dfd6b54ce33960e420fb206d8872fb759b577ad9
+
+From dfd6b54ce33960e420fb206d8872fb759b577ad9 Mon Sep 17 00:00:00 2001
+From: Tim Kientzle <kientzle@acm.org>
+Date: Sun, 11 Sep 2016 13:21:57 -0700
+Subject: [PATCH] Fixes for Issue #745 and Issue #746 from Doran Moppert.
+
+---
+ libarchive/archive_write_disk_posix.c | 294 ++++++++++++++++++++++++++--------
+ 1 file changed, 227 insertions(+), 67 deletions(-)
+
+diff --git a/libarchive/archive_write_disk_posix.c b/libarchive/archive_write_disk_posix.c
+index 8f0421e..abe1a86 100644
+--- a/libarchive/archive_write_disk_posix.c
++++ b/libarchive/archive_write_disk_posix.c
+@@ -326,12 +326,14 @@ struct archive_write_disk {
+
+ #define HFS_BLOCKS(s) ((s) >> 12)
+
++static int check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags);
+ static int check_symlinks(struct archive_write_disk *);
+ static int create_filesystem_object(struct archive_write_disk *);
+ static struct fixup_entry *current_fixup(struct archive_write_disk *, const char *pathname);
+ #if defined(HAVE_FCHDIR) && defined(PATH_MAX)
+ static void edit_deep_directories(struct archive_write_disk *ad);
+ #endif
++static int cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags);
+ static int cleanup_pathname(struct archive_write_disk *);
+ static int create_dir(struct archive_write_disk *, char *);
+ static int create_parent_dir(struct archive_write_disk *, char *);
+@@ -2014,6 +2016,10 @@ create_filesystem_object(struct archive_write_disk *a)
+ const char *linkname;
+ mode_t final_mode, mode;
+ int r;
++ /* these for check_symlinks_fsobj */
++ char *linkname_copy; /* non-const copy of linkname */
++ struct archive_string error_string;
++ int error_number;
+
+ /* We identify hard/symlinks according to the link names. */
+ /* Since link(2) and symlink(2) don't handle modes, we're done here. */
+@@ -2022,6 +2028,27 @@ create_filesystem_object(struct archive_write_disk *a)
+ #if !HAVE_LINK
+ return (EPERM);
+ #else
++ archive_string_init(&error_string);
++ linkname_copy = strdup(linkname);
++ if (linkname_copy == NULL) {
++ return (EPERM);
++ }
++ /* TODO: consider using the cleaned-up path as the link target? */
++ r = cleanup_pathname_fsobj(linkname_copy, &error_number, &error_string, a->flags);
++ if (r != ARCHIVE_OK) {
++ archive_set_error(&a->archive, error_number, "%s", error_string.s);
++ free(linkname_copy);
++ /* EPERM is more appropriate than error_number for our callers */
++ return (EPERM);
++ }
++ r = check_symlinks_fsobj(linkname_copy, &error_number, &error_string, a->flags);
++ if (r != ARCHIVE_OK) {
++ archive_set_error(&a->archive, error_number, "%s", error_string.s);
++ free(linkname_copy);
++ /* EPERM is more appropriate than error_number for our callers */
++ return (EPERM);
++ }
++ free(linkname_copy);
+ r = link(linkname, a->name) ? errno : 0;
+ /*
+ * New cpio and pax formats allow hardlink entries
+@@ -2362,115 +2389,228 @@ current_fixup(struct archive_write_disk *a, const char *pathname)
+ * recent paths.
+ */
+ /* TODO: Extend this to support symlinks on Windows Vista and later. */
++
++/*
++ * Checks the given path to see if any elements along it are symlinks. Returns
++ * ARCHIVE_OK if there are none, otherwise puts an error in errmsg.
++ */
+ static int
+-check_symlinks(struct archive_write_disk *a)
++check_symlinks_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags)
+ {
+ #if !defined(HAVE_LSTAT)
+ /* Platform doesn't have lstat, so we can't look for symlinks. */
+ (void)a; /* UNUSED */
++ (void)path; /* UNUSED */
++ (void)error_number; /* UNUSED */
++ (void)error_string; /* UNUSED */
++ (void)flags; /* UNUSED */
+ return (ARCHIVE_OK);
+ #else
+- char *pn;
++ int res = ARCHIVE_OK;
++ char *tail;
++ char *head;
++ int last;
+ char c;
+ int r;
+ struct stat st;
++ int restore_pwd;
++
++ /* Nothing to do here if name is empty */
++ if(path[0] == '\0')
++ return (ARCHIVE_OK);
+
+ /*
+ * Guard against symlink tricks. Reject any archive entry whose
+ * destination would be altered by a symlink.
++ *
++ * Walk the filename in chunks separated by '/'. For each segment:
++ * - if it doesn't exist, continue
++ * - if it's symlink, abort or remove it
++ * - if it's a directory and it's not the last chunk, cd into it
++ * As we go:
++ * head points to the current (relative) path
++ * tail points to the temporary \0 terminating the segment we're currently examining
++ * c holds what used to be in *tail
++ * last is 1 if this is the last tail
+ */
+- /* Whatever we checked last time doesn't need to be re-checked. */
+- pn = a->name;
+- if (archive_strlen(&(a->path_safe)) > 0) {
+- char *p = a->path_safe.s;
+- while ((*pn != '\0') && (*p == *pn))
+- ++p, ++pn;
+- }
++ restore_pwd = open(".", O_RDONLY | O_BINARY | O_CLOEXEC);
++ __archive_ensure_cloexec_flag(restore_pwd);
++ if (restore_pwd < 0)
++ return (ARCHIVE_FATAL);
++ head = path;
++ tail = path;
++ last = 0;
++ /* TODO: reintroduce a safe cache here? */
+ /* Skip the root directory if the path is absolute. */
+- if(pn == a->name && pn[0] == '/')
+- ++pn;
+- c = pn[0];
+- /* Keep going until we've checked the entire name. */
+- while (pn[0] != '\0' && (pn[0] != '/' || pn[1] != '\0')) {
++ if(tail == path && tail[0] == '/')
++ ++tail;
++ /* Keep going until we've checked the entire name.
++ * head, tail, path all alias the same string, which is
++ * temporarily zeroed at tail, so be careful restoring the
++ * stashed (c=tail[0]) for error messages.
++ * Exiting the loop with break is okay; continue is not.
++ */
++ while (!last) {
++ /* Skip the separator we just consumed, plus any adjacent ones */
++ while (*tail == '/')
++ ++tail;
+ /* Skip the next path element. */
+- while (*pn != '\0' && *pn != '/')
+- ++pn;
+- c = pn[0];
+- pn[0] = '\0';
++ while (*tail != '\0' && *tail != '/')
++ ++tail;
++ /* is this the last path component? */
++ last = (tail[0] == '\0') || (tail[0] == '/' && tail[1] == '\0');
++ /* temporarily truncate the string here */
++ c = tail[0];
++ tail[0] = '\0';
+ /* Check that we haven't hit a symlink. */
+- r = lstat(a->name, &st);
++ r = lstat(head, &st);
+ if (r != 0) {
++ tail[0] = c;
+ /* We've hit a dir that doesn't exist; stop now. */
+ if (errno == ENOENT) {
+ break;
+ } else {
+- /* Note: This effectively disables deep directory
++ /* Treat any other error as fatal - best to be paranoid here
++ * Note: This effectively disables deep directory
+ * support when security checks are enabled.
+ * Otherwise, very long pathnames that trigger
+ * an error here could evade the sandbox.
+ * TODO: We could do better, but it would probably
+ * require merging the symlink checks with the
+ * deep-directory editing. */
+- return (ARCHIVE_FAILED);
++ if (error_number) *error_number = errno;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Could not stat %s",
++ path);
++ res = ARCHIVE_FAILED;
++ break;
++ }
++ } else if (S_ISDIR(st.st_mode)) {
++ if (!last) {
++ if (chdir(head) != 0) {
++ tail[0] = c;
++ if (error_number) *error_number = errno;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Could not chdir %s",
++ path);
++ res = (ARCHIVE_FATAL);
++ break;
++ }
++ /* Our view is now from inside this dir: */
++ head = tail + 1;
+ }
+ } else if (S_ISLNK(st.st_mode)) {
+- if (c == '\0') {
++ if (last) {
+ /*
+ * Last element is symlink; remove it
+ * so we can overwrite it with the
+ * item being extracted.
+ */
+- if (unlink(a->name)) {
+- archive_set_error(&a->archive, errno,
+- "Could not remove symlink %s",
+- a->name);
+- pn[0] = c;
+- return (ARCHIVE_FAILED);
++ if (unlink(head)) {
++ tail[0] = c;
++ if (error_number) *error_number = errno;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Could not remove symlink %s",
++ path);
++ res = ARCHIVE_FAILED;
++ break;
+ }
+- a->pst = NULL;
+ /*
+ * Even if we did remove it, a warning
+ * is in order. The warning is silly,
+ * though, if we're just replacing one
+ * symlink with another symlink.
+ */
+- if (!S_ISLNK(a->mode)) {
+- archive_set_error(&a->archive, 0,
+- "Removing symlink %s",
+- a->name);
++ tail[0] = c;
++ /* FIXME: not sure how important this is to restore
++ if (!S_ISLNK(path)) {
++ if (error_number) *error_number = 0;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Removing symlink %s",
++ path);
+ }
++ */
+ /* Symlink gone. No more problem! */
+- pn[0] = c;
+- return (0);
+- } else if (a->flags & ARCHIVE_EXTRACT_UNLINK) {
++ res = ARCHIVE_OK;
++ break;
++ } else if (flags & ARCHIVE_EXTRACT_UNLINK) {
+ /* User asked us to remove problems. */
+- if (unlink(a->name) != 0) {
+- archive_set_error(&a->archive, 0,
+- "Cannot remove intervening symlink %s",
+- a->name);
+- pn[0] = c;
+- return (ARCHIVE_FAILED);
++ if (unlink(head) != 0) {
++ tail[0] = c;
++ if (error_number) *error_number = 0;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Cannot remove intervening symlink %s",
++ path);
++ res = ARCHIVE_FAILED;
++ break;
+ }
+- a->pst = NULL;
++ tail[0] = c;
+ } else {
+- archive_set_error(&a->archive, 0,
+- "Cannot extract through symlink %s",
+- a->name);
+- pn[0] = c;
+- return (ARCHIVE_FAILED);
++ tail[0] = c;
++ if (error_number) *error_number = 0;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Cannot extract through symlink %s",
++ path);
++ res = ARCHIVE_FAILED;
++ break;
+ }
+ }
+- pn[0] = c;
+- if (pn[0] != '\0')
+- pn++; /* Advance to the next segment. */
++ /* be sure to always maintain this */
++ tail[0] = c;
++ if (tail[0] != '\0')
++ tail++; /* Advance to the next segment. */
+ }
+- pn[0] = c;
+- /* We've checked and/or cleaned the whole path, so remember it. */
+- archive_strcpy(&a->path_safe, a->name);
+- return (ARCHIVE_OK);
++ /* Catches loop exits via break */
++ tail[0] = c;
++#ifdef HAVE_FCHDIR
++ /* If we changed directory above, restore it here. */
++ if (restore_pwd >= 0) {
++ r = fchdir(restore_pwd);
++ if (r != 0) {
++ if(error_number) *error_number = errno;
++ if(error_string)
++ archive_string_sprintf(error_string,
++ "chdir() failure");
++ }
++ close(restore_pwd);
++ restore_pwd = -1;
++ if (r != 0) {
++ res = (ARCHIVE_FATAL);
++ }
++ }
++#endif
++ /* TODO: reintroduce a safe cache here? */
++ return res;
+ #endif
+ }
+
++/*
++ * Check a->name for symlinks, returning ARCHIVE_OK if its clean, otherwise
++ * calls archive_set_error and returns ARCHIVE_{FATAL,FAILED}
++ */
++static int
++check_symlinks(struct archive_write_disk *a)
++{
++ struct archive_string error_string;
++ int error_number;
++ int rc;
++ archive_string_init(&error_string);
++ rc = check_symlinks_fsobj(a->name, &error_number, &error_string, a->flags);
++ if (rc != ARCHIVE_OK) {
++ archive_set_error(&a->archive, error_number, "%s", error_string.s);
++ }
++ archive_string_free(&error_string);
++ a->pst = NULL; /* to be safe */
++ return rc;
++}
++
++
+ #if defined(__CYGWIN__)
+ /*
+ * 1. Convert a path separator from '\' to '/' .
+@@ -2544,15 +2684,17 @@ cleanup_pathname_win(struct archive_write_disk *a)
+ * is set) if the path is absolute.
+ */
+ static int
+-cleanup_pathname(struct archive_write_disk *a)
++cleanup_pathname_fsobj(char *path, int *error_number, struct archive_string *error_string, int flags)
+ {
+ char *dest, *src;
+ char separator = '\0';
+
+- dest = src = a->name;
++ dest = src = path;
+ if (*src == '\0') {
+- archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+- "Invalid empty pathname");
++ if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Invalid empty pathname");
+ return (ARCHIVE_FAILED);
+ }
+
+@@ -2561,9 +2703,11 @@ cleanup_pathname(struct archive_write_disk *a)
+ #endif
+ /* Skip leading '/'. */
+ if (*src == '/') {
+- if (a->flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
+- archive_set_error(&a->archive, ARCHIVE_ERRNO_MISC,
+- "Path is absolute");
++ if (flags & ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS) {
++ if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Path is absolute");
+ return (ARCHIVE_FAILED);
+ }
+
+@@ -2590,10 +2734,11 @@ cleanup_pathname(struct archive_write_disk *a)
+ } else if (src[1] == '.') {
+ if (src[2] == '/' || src[2] == '\0') {
+ /* Conditionally warn about '..' */
+- if (a->flags & ARCHIVE_EXTRACT_SECURE_NODOTDOT) {
+- archive_set_error(&a->archive,
+- ARCHIVE_ERRNO_MISC,
+- "Path contains '..'");
++ if (flags & ARCHIVE_EXTRACT_SECURE_NODOTDOT) {
++ if (error_number) *error_number = ARCHIVE_ERRNO_MISC;
++ if (error_string)
++ archive_string_sprintf(error_string,
++ "Path contains '..'");
+ return (ARCHIVE_FAILED);
+ }
+ }
+@@ -2624,7 +2769,7 @@ cleanup_pathname(struct archive_write_disk *a)
+ * We've just copied zero or more path elements, not including the
+ * final '/'.
+ */
+- if (dest == a->name) {
++ if (dest == path) {
+ /*
+ * Nothing got copied. The path must have been something
+ * like '.' or '/' or './' or '/././././/./'.
+@@ -2639,6 +2784,21 @@ cleanup_pathname(struct archive_write_disk *a)
+ return (ARCHIVE_OK);
+ }
+
++static int
++cleanup_pathname(struct archive_write_disk *a)
++{
++ struct archive_string error_string;
++ int error_number;
++ int rc;
++ archive_string_init(&error_string);
++ rc = cleanup_pathname_fsobj(a->name, &error_number, &error_string, a->flags);
++ if (rc != ARCHIVE_OK) {
++ archive_set_error(&a->archive, error_number, "%s", error_string.s);
++ }
++ archive_string_free(&error_string);
++ return rc;
++}
++
+ /*
+ * Create the parent directory of the specified path, assuming path
+ * is already in mutable storage.
diff --git a/gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch b/gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch
new file mode 100644
index 0000000..0e70ac9
--- /dev/null
+++ b/gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch
@@ -0,0 +1,44 @@
+Fixes this buffer overflow:
+https://github.com/libarchive/libarchive/commit/e37b620fe8f14535d737e89a4dcabaed4517bf1a
+
+Patch copied from upstream source repository:
+https://github.com/libarchive/libarchive/commit/e37b620fe8f14535d737e89a4dcabaed4517bf1a
+
+From e37b620fe8f14535d737e89a4dcabaed4517bf1a Mon Sep 17 00:00:00 2001
+From: Tim Kientzle <kientzle@acm.org>
+Date: Sun, 21 Aug 2016 10:51:43 -0700
+Subject: [PATCH] Issue #767: Buffer overflow printing a filename
+
+The safe_fprintf function attempts to ensure clean output for an
+arbitrary sequence of bytes by doing a trial conversion of the
+multibyte characters to wide characters -- if the resulting wide
+character is printable then we pass through the corresponding bytes
+unaltered, otherwise, we convert them to C-style ASCII escapes.
+
+The stack trace in Issue #767 suggest that the 20-byte buffer
+was getting overflowed trying to format a non-printable multibyte
+character. This should only happen if there is a valid multibyte
+character of more than 5 bytes that was unprintable. (Each byte
+would get expanded to a four-charcter octal-style escape of the form
+"\123" resulting in >20 characters for the >5 byte multibyte character.)
+
+I've not been able to reproduce this, but have expanded the conversion
+buffer to 128 bytes on the belief that no multibyte character set
+has a single character of more than 32 bytes.
+---
+ tar/util.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/tar/util.c b/tar/util.c
+index 9ff22f2..2b4aebe 100644
+--- a/tar/util.c
++++ b/tar/util.c
+@@ -182,7 +182,7 @@ safe_fprintf(FILE *f, const char *fmt, ...)
+ }
+
+ /* If our output buffer is full, dump it and keep going. */
+- if (i > (sizeof(outbuff) - 20)) {
++ if (i > (sizeof(outbuff) - 128)) {
+ outbuff[i] = '\0';
+ fprintf(f, "%s", outbuff);
+ i = 0;
--
2.10.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: libarchive security fixes (was Re: Core-updates timeline)
2016-10-02 20:14 ` libarchive security fixes (was Re: Core-updates timeline) Leo Famulari
@ 2016-10-03 16:10 ` Ludovic Courtès
2016-10-03 18:14 ` Leo Famulari
0 siblings, 1 reply; 15+ messages in thread
From: Ludovic Courtès @ 2016-10-03 16:10 UTC (permalink / raw)
To: Leo Famulari; +Cc: guix-devel
Hi!
Leo Famulari <leo@famulari.name> skribis:
> On Sun, Oct 02, 2016 at 02:50:34PM -0400, Leo Famulari wrote:
>> On Sun, Oct 02, 2016 at 03:38:58PM +0200, Ludovic Courtès wrote:
>> > We could wait an additional day for libarchive if it’s more convenient,
>> > but maybe not longer than that.
>> >
>> > What do you think would be the most convenient approach?
>>
>> I will send a patch that cherry-picks what I think are the most
>> important bug fixes. I can't guess when libarchive 3.2.2 will be
>> released.
>
> I've attached a patch.
>
> It cherry-picks some fixes for some filesystem attacks and two overflows
> that can be triggered with "crafted" input. The details are in the patch
> files.
>
> I understand if this approach of cherry-picking a handful of commits is
> not acceptable. It's hard to judge the full impact of taking only these
> changes, some of which a quite significant, without being familiar with
> the libarchive code.
>
> That's the reason why I've been waiting for a new upstream release. But
> I figured I should at least try to get these bug fixes into the next
> release of Guix :)
Sounds reasonable. :-)
> From 042d5a7df4962c3b81fbfefa0027b6f1cf356b5f Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo@famulari.name>
> Date: Sun, 2 Oct 2016 15:58:06 -0400
> Subject: [PATCH] gnu: libarchive: Fix several security issues.
>
> * gnu/packages/backup.scm (libarchive)[replacement]: New field.
> (libarchive/fixed): New variable.
> * gnu/packages/patches/libarchive-7zip-heap-overflow.patch,
> gnu/packages/patches/libarchive-fix-symlink-check.patch,
> gnu/packages/patches/libarchive-fix-filesystem-attacks.patch,
> gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch: New files.
> * gnu/local.mk (dist_patch_DATA): Add them.
Don’t they have a CVE assigned? If so, please make sure to name them
accordingly. Otherwise LGTM.
I won’t pretend to have a precise understanding of the impact of these
bugs, but clearly they can be triggered with specially-crafted input,
which sounds bad. So better have these fixes.
Thank you!
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: libarchive security fixes (was Re: Core-updates timeline)
2016-10-03 16:10 ` Ludovic Courtès
@ 2016-10-03 18:14 ` Leo Famulari
0 siblings, 0 replies; 15+ messages in thread
From: Leo Famulari @ 2016-10-03 18:14 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
On Mon, Oct 03, 2016 at 06:10:10PM +0200, Ludovic Courtès wrote:
> Leo Famulari <leo@famulari.name> skribis:
> > I understand if this approach of cherry-picking a handful of commits is
> > not acceptable. It's hard to judge the full impact of taking only these
> > changes, some of which a quite significant, without being familiar with
> > the libarchive code.
> >
> > That's the reason why I've been waiting for a new upstream release. But
> > I figured I should at least try to get these bug fixes into the next
> > release of Guix :)
>
> Sounds reasonable. :-)
Okay, as long as the patch itself is reasonable :)
> > Subject: [PATCH] gnu: libarchive: Fix several security issues.
> >
> > * gnu/packages/backup.scm (libarchive)[replacement]: New field.
> > (libarchive/fixed): New variable.
> > * gnu/packages/patches/libarchive-7zip-heap-overflow.patch,
> > gnu/packages/patches/libarchive-fix-symlink-check.patch,
> > gnu/packages/patches/libarchive-fix-filesystem-attacks.patch,
> > gnu/packages/patches/libarchive-safe_fprintf-buffer-overflow.patch: New files.
> > * gnu/local.mk (dist_patch_DATA): Add them.
>
> Don’t they have a CVE assigned? If so, please make sure to name them
> accordingly. Otherwise LGTM.
Not AFAICT, despite the fact that they have all been sent to the oss-sec
mailing list.
Both of the overflow bugs were reported here:
http://seclists.org/oss-sec/2016/q3/516
And the filesystem attacks:
http://seclists.org/oss-sec/2016/q3/255
> I won’t pretend to have a precise understanding of the impact of these
> bugs, but clearly they can be triggered with specially-crafted input,
> which sounds bad. So better have these fixes.
My understand is the the filesystem and symlink bugs allow the creator
of the archive being parsed by libarchive to overwrite any file on the
target system due to a set of bugs related to symlink checking, via a
variety of mechanisms (detailed explanations are linked to from the
patch files).
The "safe_printf" patch corrects a stack overflow triggered by very
large multibyte characters in filenames to-be-printed. This is under the
control of whoever creates the archive file.
And the 7zip patch corrects a heap overflow when reading crafted 7zip
archives. Again, this is something the attacker can trigger.
I don't know if these two overflows are "exploitable" or not.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Core-updates timeline
2016-10-02 13:38 ` Core-updates timeline Ludovic Courtès
2016-10-02 18:50 ` Leo Famulari
@ 2016-10-07 20:16 ` Ludovic Courtès
1 sibling, 0 replies; 15+ messages in thread
From: Ludovic Courtès @ 2016-10-07 20:16 UTC (permalink / raw)
To: Leo Famulari; +Cc: guix-devel
Hello!
I’ve just started an evaluation of all the packages of ‘core-updates’.
So now the only changes allowed there are fixes!
https://hydra.gnu.org/jobset/gnu/core-updates
If at some point we need to fix a core package, we can always cancel all
the scheduled builds and restart an evaluation.
Ludo’.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-10-07 20:17 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-20 4:56 [PATCH 0/2] Perl: Enable threading support Ben Woodcroft
2016-09-20 4:56 ` [PATCH 1/2] gnu: perl: Split configure phase Ben Woodcroft
2016-09-24 5:02 ` Ludovic Courtès
2016-09-20 4:56 ` [PATCH 2/2] gnu: perl: Enable threading support Ben Woodcroft
2016-09-24 5:05 ` Ludovic Courtès
2016-09-26 10:03 ` Ben Woodcroft
2016-10-01 13:22 ` Ludovic Courtès
2016-10-01 16:40 ` Core-updates timeline (was: Re: [PATCH 2/2] gnu: perl: Enable threading support.) Leo Famulari
2016-10-02 13:38 ` Core-updates timeline Ludovic Courtès
2016-10-02 18:50 ` Leo Famulari
2016-10-02 20:14 ` libarchive security fixes (was Re: Core-updates timeline) Leo Famulari
2016-10-03 16:10 ` Ludovic Courtès
2016-10-03 18:14 ` Leo Famulari
2016-10-07 20:16 ` Core-updates timeline Ludovic Courtès
2016-09-20 20:58 ` [PATCH 0/2] Perl: Enable threading support Eric Bavier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).