From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark H Weaver Subject: bug#24703: Store references in 8-byte chunks in compiled code Date: Mon, 31 Oct 2016 02:35:49 -0400 Message-ID: <87oa21c9fe.fsf@netris.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> <87shrunicp.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:49022) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c16EQ-0001bB-7X for bug-guix@gnu.org; Mon, 31 Oct 2016 02:38:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c16EM-0005Pi-6e for bug-guix@gnu.org; Mon, 31 Oct 2016 02:38:06 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:50427) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1c16EM-0005Pb-3M for bug-guix@gnu.org; Mon, 31 Oct 2016 02:38:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87shrunicp.fsf@gnu.org> ("Ludovic \=\?utf-8\?Q\?Court\=C3\=A8s\=22'\?\= \=\?utf-8\?Q\?s\?\= message of "Tue, 18 Oct 2016 10:59:02 +0200") 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: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 24703@debbugs.gnu.org ludo@gnu.org (Ludovic Court=C3=A8s) writes: > Mark H Weaver skribis: > >> 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 b= uild-system gnu)). Indeed. The derivations that crashed were using a grafted 'guile'. These were not 'gnu-build' derivations, but simpler derivations such as 'module-import-compiled' derivations and some others involved with building a 'system'. >> 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_movs= tr=E2=80=99 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 > #endif >=20=20 > +/* Return true if STR is a string denoting a "/gnu/store" file name. */ > + > +static bool > +store_reference_p (tree str) > +{ > + const char *store; > + > + store =3D getenv ("NIX_STORE") ?: "/gnu/store"; > + > + return (TREE_STRING_LENGTH (str) > strlen (store) > + && strncmp (TREE_STRING_POINTER (str), store, strlen (store))); > +} [...] > WDYT? I think it's not sufficient to apply this workaround only for string literals that _begin_ with the store directory. In some cases, the store name may appear only in the middle of a string. > 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: I consider it a potentially serious security problem that ungrafted outputs are being used. Papering over the problem by preventing this buggy software from being deleted is, in my opinion, not acceptable. I would suggest instead that we'll need to add grafts for all packages that include these chunked references. However, due to bug 24832, it may be that we'll need to rebuild all of 'core-updates' from scratch anyway. >> 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 compil= ation > tweak. > > WDYT? The GCC approach is okay with me in the short term, but I'll likely want to revisit this issue in the future. Thanks, Mark