From: ludo@gnu.org (Ludovic Courtès)
To: Mark H Weaver <mhw@netris.org>
Cc: 24703@debbugs.gnu.org
Subject: bug#24703: Store references in 8-byte chunks in compiled code
Date: Tue, 18 Oct 2016 10:59:02 +0200 [thread overview]
Message-ID: <87shrunicp.fsf@gnu.org> (raw)
In-Reply-To: <87k2d6qqee.fsf@netris.org> (Mark H. Weaver's message of "Mon, 17 Oct 2016 23:36:57 -0400")
[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]
Mark H Weaver <mhw@netris.org> skribis:
> ludo@gnu.org (Ludovic Courtès) writes:
[...]
>> We could use a self symlink, or we could use something like
>> <http://git.savannah.gnu.org/cgit/guix.git/commit/?id=9d50da70608de32d9db0c29859caec6f2cddb95f>.
>>
>> 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
>
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24712#13
>
> 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):
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1155 bytes --]
--- 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
#endif
+/* 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
+ <http://bugs.gnu.org/24703> for background. */
+ if (!HAVE_movstr || store_reference_p (src))
return NULL_RTX;
dest_mem = get_memory_rtx (dest, NULL);
[-- Attachment #3: Type: text/plain, Size: 160 bytes --]
WDYT?
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:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: Type: text/x-patch, Size: 696 bytes --]
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."
files))
(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" <>))
[-- Attachment #5: Type: text/plain, Size: 712 bytes --]
> 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
tweak.
WDYT?
Thanks,
Ludo’.
next prev parent reply other threads:[~2016-10-18 9:00 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-16 3:49 bug#24703: fontconfig keeps obfuscated reference to itself, not grafted Mark H Weaver
2016-10-16 4:26 ` Mark H Weaver
2016-10-16 5:00 ` Mark H Weaver
2016-10-16 6:24 ` bug#24703: Store references in 8-byte chunks in compiled code Mark H Weaver
2016-10-16 9:03 ` Mark H Weaver
2016-10-16 9:25 ` Mark H Weaver
2016-10-16 10:15 ` Mark H Weaver
2016-10-16 19:04 ` Ludovic Courtès
2016-10-17 7:46 ` bug#24703: " Török Edwin
2016-10-17 9:42 ` Mark H Weaver
2016-10-17 12:09 ` Ludovic Courtès
2016-10-18 3:36 ` Mark H Weaver
2016-10-18 8:59 ` Ludovic Courtès [this message]
2016-10-31 6:35 ` Mark H Weaver
2016-10-31 11:37 ` Ludovic Courtès
2016-10-24 19:40 ` Leo Famulari
2016-10-24 20:18 ` Ludovic Courtès
2016-11-04 23:15 ` Ludovic Courtès
2016-11-05 18:36 ` Leo Famulari
2016-11-06 20:58 ` Ludovic Courtès
2016-11-09 20:40 ` Ludovic Courtès
2016-11-09 23:16 ` Leo Famulari
2016-11-10 8:01 ` Ludovic Courtès
2017-04-02 22:19 ` Ludovic Courtès
2016-11-11 10:39 ` Ludovic Courtès
2016-10-19 21:25 ` Török Edwin
2016-10-20 12:25 ` Ludovic Courtès
2016-10-16 14:42 ` bug#24703: fontconfig keeps obfuscated reference to itself, not grafted Ludovic Courtès
2016-10-16 15:06 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87shrunicp.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=24703@debbugs.gnu.org \
--cc=mhw@netris.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.