Guile-provided GMP allocators interfere with GnuTLS

DoneSubmitted by Ludovic Courtès.
Details
2 participants
  • Ludovic Courtès
  • Marius Bakke
Owner
unassigned
Severity
serious
L
L
Ludovic Courtès wrote on 5 Feb 2021 17:59
Download
(address . bug-guix@gnu.org)
87v9b61md4.fsf@inria.fr
Download
Fellow Debian hackers identified a bug causing memory corruption in
Nettle data structures used by GnuTLS when GnuTLS is used from Guile:

https://bugs.debian.org/964284

In a nutshell, Guile installs its own GMP memory allocation routines
(when ‘scm_install_gmp_memory_functions’ is true, which is the default)
so that GMP allocates via libgc. GnuTLS uses Nettle, which uses GMP, so
Nettle too ends up allocating via libgc; however, since pointers to that
memory are not scanned by libgc, they end up being reclaimed early.

In practice, memory corruption is relatively rare, to the point that we
did not notice it in Guix. In Debian, it would lead to a failure of the
‘tests/reauth.scm’ test in GnuTLS. With minor modifications to the
test, as noted in the thread above, I can reproduce it on Guix as well.

The thread above mentions possible workaround, but there’s nothing
satisfactory.

The longer-term solution is to use mini-GMP in Guile (which is also nice
as a way to reduce dependencies).

To be continued…

Ludo’.
L
L
Ludovic Courtès wrote on 5 Feb 2021 18:01
Download
control message for bug #46330
(address . control@debbugs.gnu.org)
87tuqq1ma3.fsf@gnu.org
Download
severity 46330 serious
quit
L
L
Ludovic Courtès wrote on 5 Feb 2021 18:13
Download
Re: bug#46330: Guile-provided GMP allocators interfere with GnuTLS
(address . 46330@debbugs.gnu.org)
87im761lr5.fsf@gnu.org
Download
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (6 lines)
> In a nutshell, Guile installs its own GMP memory allocation routines
> (when ‘scm_install_gmp_memory_functions’ is true, which is the default)
> so that GMP allocates via libgc. GnuTLS uses Nettle, which uses GMP, so
> Nettle too ends up allocating via libgc; however, since pointers to that
> memory are not scanned by libgc, they end up being reclaimed early.

One of the solutions is to set:

scm_install_gmp_memory_functions = 0;

in Guile, as Andy suggested on IRC, but it incurs a performance hit on
bignum-heavy applications such as the compiler:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964284#78

However, since Guix now uses its own ‘guile’ binary, we can work around
the issue like so:
Download
Toggle diff (23 lines)
diff --git a/gnu/packages/aux-files/guile-launcher.c b/gnu/packages/aux-files/guile-launcher.c
index 1dd5d77e66..814084e032 100644
--- a/gnu/packages/aux-files/guile-launcher.c
+++ b/gnu/packages/aux-files/guile-launcher.c
@@ -1,5 +1,5 @@
 /* GNU Guix --- Functional package management for GNU
-   Copyright 1996-1997,2000-2001,2006,2008,2011,2013,2018,2020
+   Copyright 1996-1997,2000-2001,2006,2008,2011,2013,2018,2020,2021
       Free Software Foundation, Inc.
    Copyright (C) 2020 Ludovic Courtès <ludo@gnu.org>
 
@@ -82,7 +82,10 @@ main (int argc, char **argv)
   unsetenv ("GUILE_LOAD_PATH");
   unsetenv ("GUILE_LOAD_COMPILED_PATH");
 
-  scm_install_gmp_memory_functions = 1;
+  /* XXX: Do not let GMP allocate via libgc as this can lead to memory
+     corruption in GnuTLS/Nettle: <https://issues.guix.gnu.org/46330>.  */
+  scm_install_gmp_memory_functions = 0;
+
   scm_boot_guile (argc, argv, inner_main, 0);
   return 0; /* never reached */
 }
Download
The advantage of this hack is that we still get to use upstream ‘guile’
for compilation purposes (with no performance hit), and we use our own
“safe” ‘guile’ executable for stuff that may use GnuTLS, in particular
‘guix substitute’ and ‘guix perform-download’.

There may still be a few cases where we’d use stock ‘guile’ together
with GnuTLS. The only example that comes to mind is when calling
‘download-nar’ or ‘swh-download’ as a fallback in (guix git-download).
That’s quite rare though.

So I think that the above is a workaround we could deploy right away.
It should allow us to wait until we have Guile on mini-GMP.

Thoughts?

Ludo’.
L
L
Ludovic Courtès wrote on 7 Feb 2021 22:24
Download
(address . 46330@debbugs.gnu.org)
87sg67y3iz.fsf@gnu.org
Download
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (6 lines)
> In a nutshell, Guile installs its own GMP memory allocation routines
> (when ‘scm_install_gmp_memory_functions’ is true, which is the default)
> so that GMP allocates via libgc. GnuTLS uses Nettle, which uses GMP, so
> Nettle too ends up allocating via libgc; however, since pointers to that
> memory are not scanned by libgc, they end up being reclaimed early.

For the record, one option I considered is to link GnuTLS against
Nettle-with-mini-GMP, whereby Nettle uses a bundled mini-GMP instead of
mini-GMP.

Another option is to build GnuTLS --with-nettle-mini, where GnuTLS links
against a bundled mini-Nettle, instead linked against a bundled mini-GMP (!).

Currently both options lead to build failures in GnuTLS:

https://lists.gnutls.org/pipermail/gnutls-help/2021-February/004680.html

(Package definitions attached for posterity.)

Ludo’.
Download
Toggle diff (75 lines)
diff --git a/gnu/packages/nettle.scm b/gnu/packages/nettle.scm
index f5e7188ff0..875a858946 100644
--- a/gnu/packages/nettle.scm
+++ b/gnu/packages/nettle.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2015, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2017 Efraim Flashner <efraim@flashner.co.il>
 ;;;
@@ -89,3 +89,12 @@ themselves.")
         ;; Build "fat" binaries where the right implementation is chosen
         ;; at run time based on CPU features (starting from 3.1.)
         `(cons "--enable-fat" ,flags))))))
+
+(define-public nettle/mini-gmp
+  (package/inherit nettle
+    (name "nettle-mini-gmp")
+    (arguments
+     (substitute-keyword-arguments (package-arguments nettle)
+       ((#:configure-flags flags)
+        `(cons "--enable-mini-gmp" ,flags))))
+    (propagated-inputs '())))
diff --git a/gnu/packages/tls.scm b/gnu/packages/tls.scm
index 775e915534..fe2ec88a9e 100644
--- a/gnu/packages/tls.scm
+++ b/gnu/packages/tls.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2014, 2015, 2016, 2017, 2018 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2014 Ian Denhardt <ian@zenhack.net>
 ;;; Copyright © 2013, 2015 Andreas Enge <andreas@enge.fr>
@@ -165,6 +165,7 @@ living in the same process.")
   (package
     (name "gnutls")
     (version "3.6.15")
+    (replacement gnutls/mini-nettle)
     (source (origin
               (method url-fetch)
               ;; Note: Releases are no longer on ftp.gnu.org since the
@@ -256,6 +257,31 @@ required structures.")
     (properties '((ftp-server . "ftp.gnutls.org")
                   (ftp-directory . "/gcrypt/gnutls")))))
 
+;; (define-public gnutls/nettle-mini-gmp
+;;   (package/inherit gnutls
+;;     (arguments
+;;      (substitute-keyword-arguments (package-arguments gnutls)
+;;        ((#:phases phases '%standard-phases)
+;;         `(modify-phases ,phases
+;;            (add-before 'configure 'dont-link-against-libgmp
+;;              (lambda _
+;;                (substitute* "configure"
+;;                  (("GMP_LIBS=\"-lgmp\"")
+;;                   "GMP_LIBS=\"\""))))))))
+;;     (propagated-inputs
+;;      `(("nettle" ,nettle/mini-gmp)
+;;        ,@(alist-delete "nettle" (package-propagated-inputs gnutls))))))
+
+(define-public gnutls/mini-nettle
+  (package/inherit gnutls
+    (arguments
+      (substitute-keyword-arguments (package-arguments gnutls)
+        ((#:configure-flags flags ''())
+         `(cons "--with-nettle-mini" ,flags))))
+    ;; (propagated-inputs
+    ;;  (alist-delete "nettle" (package-propagated-inputs gnutls)))
+    ))
+
 (define-public gnutls/guile-2.0
   ;; GnuTLS for Guile 2.0.
   (package/inherit gnutls
L
L
Ludovic Courtès wrote on 7 Feb 2021 23:47
Download
(address . 46330@debbugs.gnu.org)
87lfbzxzpy.fsf@gnu.org
Download
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (4 lines)
> One of the solutions is to set:
>
> scm_install_gmp_memory_functions = 0;

Done in a53f711422f63d7e32b8639b968cf00bcc69ffea, followed by an update
of the ‘guix’ package in 63d4b74420563c4e2dbdfa29b3816d1dad9cd723.

This mostly solves the problem on the Guix side, but the issue remains
in GnuTLS. I practical terms, we could experience random test failures
in the guile-gnutls test suite, like the Debian folks did.

At the very least we’ll need to work around that possibility in
‘core-updates’. We could skip them, or add ‘gc-disable’ calls there.
Or we could build GnuTLS against Nettle-with-mini-GMP when that becomes
an option.

The other option coming up is to build Guile against mini-GMP. Mike
Gran just started looked into it and it may be that 3.0.6 will offer it.

I’m keeping the bug open until this is sorted out.

Ludo’.
M
M
Marius Bakke wrote on 23 May 2021 16:47
Download
87tumtqzg3.fsf@gnu.org
Download
Ludovic Courtès <ludo@gnu.org> skriver:

Toggle quote (23 lines)
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> One of the solutions is to set:
>>
>> scm_install_gmp_memory_functions = 0;
>
> Done in a53f711422f63d7e32b8639b968cf00bcc69ffea, followed by an update
> of the ‘guix’ package in 63d4b74420563c4e2dbdfa29b3816d1dad9cd723.
>
> This mostly solves the problem on the Guix side, but the issue remains
> in GnuTLS. I practical terms, we could experience random test failures
> in the guile-gnutls test suite, like the Debian folks did.
>
> At the very least we’ll need to work around that possibility in
> ‘core-updates’. We could skip them, or add ‘gc-disable’ calls there.
> Or we could build GnuTLS against Nettle-with-mini-GMP when that becomes
> an option.
>
> The other option coming up is to build Guile against mini-GMP. Mike
> Gran just started looked into it and it may be that 3.0.6 will offer it.
>
> I’m keeping the bug open until this is sorted out.

I believe this was sorted with the mini-gmp in Guile 3.0.6. Please
reopen if I'm mistaken. :-)
Download
-----BEGIN PGP SIGNATURE-----

iIUEARYKAC0WIQRNTknu3zbaMQ2ddzTocYulkRQQdwUCYKprDA8cbWFyaXVzQGdu
dS5vcmcACgkQ6HGLpZEUEHcHMgD/cD1VlvhnW1caal1DZUSzMUltYw0qDZVG2Wq8
ObQ7QZoBALpsLvFU4bRYVq/YJuaMPT/8hdagiBJfE8B+laGKkPsM
=qAfT
-----END PGP SIGNATURE-----

Closed
L
L
Ludovic Courtès wrote on 25 May 2021 22:19
Download
(name . Marius Bakke)(address . marius@gnu.org)(address . 46330-done@debbugs.gnu.org)
87fsya1s8o.fsf@gnu.org
Download
Hi Marius,

Marius Bakke <marius@gnu.org> skribis:

Toggle quote (28 lines)
> Ludovic Courtès <ludo@gnu.org> skriver:
>
>> Ludovic Courtès <ludo@gnu.org> skribis:
>>
>>> One of the solutions is to set:
>>>
>>> scm_install_gmp_memory_functions = 0;
>>
>> Done in a53f711422f63d7e32b8639b968cf00bcc69ffea, followed by an update
>> of the ‘guix’ package in 63d4b74420563c4e2dbdfa29b3816d1dad9cd723.
>>
>> This mostly solves the problem on the Guix side, but the issue remains
>> in GnuTLS. I practical terms, we could experience random test failures
>> in the guile-gnutls test suite, like the Debian folks did.
>>
>> At the very least we’ll need to work around that possibility in
>> ‘core-updates’. We could skip them, or add ‘gc-disable’ calls there.
>> Or we could build GnuTLS against Nettle-with-mini-GMP when that becomes
>> an option.
>>
>> The other option coming up is to build Guile against mini-GMP. Mike
>> Gran just started looked into it and it may be that 3.0.6 will offer it.
>>
>> I’m keeping the bug open until this is sorted out.
>
> I believe this was sorted with the mini-gmp in Guile 3.0.6. Please
> reopen if I'm mistaken. :-)

Definitely; it’s wonderful. :-)

I adjusted ‘guile-launcher.c’ accordingly in
d92ee0a8bdc324726e737bf4ef099d75724ce8c9.

Thanks,
Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send email to 46330@debbugs.gnu.org