Mark H Weaver <> skribis:

> (Ludovic Courtès) writes:


>> We could use a self symlink, or we could use something like
>> <>.
>> Mark, WDYT?
>> What remains to be seen is how many packages are affected by this issue,
>> and whether a generic solution needs to be found.
> Unfortunately, it is too widespread.  As I just pointed out in
> Among the many packages that include these obfuscated store references,
> one is 'glibc-final'.  My attempt to graft 'bash' in 'master' to fix
> CVE-2016-0634 and CVE-2016-7543 has resulted in a system where I cannot
> build *any* derivation, because 'guile-final' crashes during boot while
> its 'glibc-final' tries to find its 'gconv' modules in the ungrafted
> 'glibc-final', which is not available in the build environment.

Out of curiosity, Guile crashes while loading iconv modules, right?

This is surprising because the guile-for-build is always the ungrafted
‘guile-final’ (see ‘gnu-build’ in (guix build-system gnu)).

> So, if our approach is to use -fno-builtin-strcpy, then we will have to
> apply it system-wide, and rebuild all of 'core-updates' from scratch.

Another approach would be to patch GCC, specifically ‘expand_movstr’ in
gcc/builtins.c, which is the part responsible for this optimization,
along these lines (untested):

--- gcc-5.3.0/gcc/builtins.c.orig	2016-10-18 10:45:35.042826368 +0200
+++ gcc-5.3.0/gcc/builtins.c	2016-10-18 10:50:46.080616285 +0200
@@ -3470,6 +3470,19 @@ expand_builtin_mempcpy_args (tree dest,
 # define CODE_FOR_movstr CODE_FOR_nothing
+/* Return true if STR is a string denoting a "/gnu/store" file name.  */
+static bool
+store_reference_p (tree str)
+  const char *store;
+  store = getenv ("NIX_STORE") ?: "/gnu/store";
+  return (TREE_STRING_LENGTH (str) > strlen (store)
+	  && strncmp (TREE_STRING_POINTER (str), store, strlen (store)));
 /* Expand into a movstr instruction, if one is available.  Return NULL_RTX if
    we failed, the caller should emit a normal call, otherwise try to
    get the result in TARGET, if convenient.  If ENDP is 0 return the
@@ -3484,7 +3497,9 @@ expand_movstr (tree dest, tree src, rtx
   rtx dest_mem;
   rtx src_mem;
-  if (!HAVE_movstr)
+  /* When SRC denotes a store item, do not perform any optimization.  See
+     <> for background.  */
+  if (!HAVE_movstr || store_reference_p (src))
     return NULL_RTX;
   dest_mem = get_memory_rtx (dest, NULL);

In the meantime, we need a workaround.  The only option I can think of
is to retain a reference to the ungrafted item by adding a symlink to
it, like:

diff --git a/guix/grafts.scm b/guix/grafts.scm
index 80ae27e..4de19ae 100644
--- a/guix/grafts.scm
+++ b/guix/grafts.scm
@@ -127,7 +127,14 @@ recursively applied to dependencies of DRV."
                    (match %outputs
                      (((names . files) ...)
-                      files))))))
+                      files)))
+         (for-each (match-lambda
+                     ((name . file)
+                      (symlink file
+                               (string-append (assoc-ref %outputs name)
+                                              "/ungrafted"))))
+                   old-outputs))))
   (define add-label
     (cut cons "x" <>))
> I've been investigating another approach: to enhance our scanner and
> grafter to handle these 8-byte-chunked references.  I believe it is
> feasible, but only if we abandon the ability to change the file names of
> grafts outside of the hash.  The reason is that the hash portion of
> store references are surrounded by enough other known characters on both
> sides that the hash portion is almost always contained entirely within
> 8-byte chunks.

I think that would add complexity, would make grafting slower, and
abandoning the ability to change file names would be a regression.

So I’m more in favor of a GCC patch like above, or another compilation



Reply via email to