From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Subject: bug#24703: Store references in 8-byte chunks in compiled code Date: Tue, 18 Oct 2016 10:59:02 +0200 Message-ID: <87shrunicp.fsf@gnu.org> References: <87mvi5lzqu.fsf@netris.org> <87inssncln.fsf@netris.org> <8737jwnb1c.fsf@netris.org> <87r37gstf6.fsf_-_@netris.org> <87d1j0sl1l.fsf@netris.org> <87a8e4glot.fsf@gnu.org> <8f2024ad-13c1-d4b1-1541-c2a5bddcb403@etorok.net> <87h98bdvng.fsf@gnu.org> <87k2d6qqee.fsf@netris.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:44958) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bwQFi-0003MJ-T1 for bug-guix@gnu.org; Tue, 18 Oct 2016 05:00:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bwQFf-0008Bz-Jc for bug-guix@gnu.org; Tue, 18 Oct 2016 05:00:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:59650) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1bwQFf-0008Bu-GJ for bug-guix@gnu.org; Tue, 18 Oct 2016 05:00:03 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87k2d6qqee.fsf@netris.org> (Mark H. Weaver's message of "Mon, 17 Oct 2016 23:36:57 -0400") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Mark H Weaver Cc: 24703@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Mark H Weaver skribis: > ludo@gnu.org (Ludovic Court=C3=A8s) 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 > > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D24712#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 =E2=80=98guile-final=E2=80=99 (see =E2=80=98gnu-build=E2=80=99 in (guix bui= ld-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 =E2=80=98expand_movstr= =E2=80=99 in gcc/builtins.c, which is the part responsible for this optimization, along these lines (untested): --=-=-= Content-Type: text/x-patch Content-Disposition: inline --- 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 + for background. */ + if (!HAVE_movstr || store_reference_p (src)) return NULL_RTX; dest_mem = get_memory_rtx (dest, NULL); --=-=-= Content-Type: text/plain 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: --=-=-= Content-Type: text/x-patch Content-Disposition: inline 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" <>)) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > 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=E2=80=99m more in favor of a GCC patch like above, or another compilat= ion tweak. WDYT? Thanks, Ludo=E2=80=99. --=-=-=--