From owner-freebsd-arch@FreeBSD.ORG Fri Aug 23 22:21:52 2013 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 0450935C for ; Fri, 23 Aug 2013 22:21:52 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (unknown [IPv6:2001:610:1108:5012::107]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 89B152865 for ; Fri, 23 Aug 2013 22:21:51 +0000 (UTC) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id 62858120139 for ; Sat, 24 Aug 2013 00:21:34 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id 3679928494; Sat, 24 Aug 2013 00:21:34 +0200 (CEST) Date: Sat, 24 Aug 2013 00:21:34 +0200 From: Jilles Tjoelker To: freebsd-arch@freebsd.org Subject: Using sys_errlist from executables is not ABI-stable Message-ID: <20130823222133.GA36931@stack.nl> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 23 Aug 2013 22:21:52 -0000 The following program crashes or produces unexpected output if compiled on stable/9 and run on head: #include #include int main(int argc, char *argv[]) { printf("Last error: %d: %s\n", sys_nerr, sys_errlist[sys_nerr - 1]); printf("Last error via strerror: %d: %s\n", sys_nerr, strerror(sys_nerr - 1)); return 0; } The problem is that ld reserves space for sys_errlist at link time. The executable accesses sys_errlist directly like its own global variables. When the executable is run, rtld copies the string pointers from libc.so.7 to the executable's global variable. It does not care that the symbol in libc.so.7 is bigger than the executable's space (due to added ENOTRECOVERABLE and EOWNERDEAD), but even if it did, it could not fix the problem anyway. Meanwhile, the value of sys_nerr reflecting the larger sys_errlist is copied unchanged. The strerror() function in libc also uses the executable's copy. In my setup, the first printf gets NULL and the strerror() call segfaults. If the executable is linked statically or compiled with -fPIE, there are no copy relocations and it does not crash. The below patch makes libc always use its own copy of sys_errlist and sys_nerr. This ensures strerror() and friends continue to work correctly when an executable imports sys_errlist, but does not fix the executable's use of sys_errlist. As a side effect, the access to sys_errlist and sys_nerr becomes slightly faster and needs less work from rtld. One way of fixing the executable's use of sys_errlist would be to export sys_nerr with the value it had in FreeBSD 7.0 (93), making __hidden_sys_nerr a separate variable with the true value (or even a #define). This ensures that no error strings will be accessed out of bounds, but only errors that were in FreeBSD 7.0 are available to applications that use this deprecated method of accessing error strings. A more "proper" method would be to symbol version sys_errlist but this still does not allow accessing error strings added after the program was linked. Also considering other problems with sys_errlist (removing it would allow storing the error messages in a way that does not create one relative relocation per error message), I don't think it is worth it. Index: lib/libc/include/errlst.h =================================================================== --- lib/libc/include/errlst.h (revision 0) +++ lib/libc/include/errlst.h (working copy) @@ -0,0 +1,37 @@ +/*- + * Copyright (c) 2013 Jilles Tjoelker + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + * + * $FreeBSD$ + */ + +#ifndef __ERRLST_H__ +#define __ERRLST_H__ + +#include + +extern const char *const __hidden_sys_errlist[] __hidden; +extern const int __hidden_sys_nerr __hidden; + +#endif /* __ERRLST_H__ */ Property changes on: lib/libc/include/errlst.h ___________________________________________________________________ Added: svn:eol-style ## -0,0 +1 ## +native \ No newline at end of property Added: svn:mime-type ## -0,0 +1 ## +text/plain \ No newline at end of property Added: svn:keywords ## -0,0 +1 ## +FreeBSD=%H \ No newline at end of property Index: lib/libc/gen/errlst.c =================================================================== --- lib/libc/gen/errlst.c (revision 254670) +++ lib/libc/gen/errlst.c (working copy) @@ -34,6 +34,7 @@ __FBSDID("$FreeBSD$"); #include +#include "errlst.h" const char *const sys_errlist[] = { "No error: 0", /* 0 - ENOERROR */ @@ -156,3 +157,6 @@ "Previous owner died", /* 96 - EOWNERDEAD */ }; const int sys_nerr = sizeof(sys_errlist) / sizeof(sys_errlist[0]); + +__strong_reference(sys_errlist, __hidden_sys_errlist); +__strong_reference(sys_nerr, __hidden_sys_nerr); Index: lib/libc/stdio/xprintf_errno.c =================================================================== --- lib/libc/stdio/xprintf_errno.c (revision 254670) +++ lib/libc/stdio/xprintf_errno.c (working copy) @@ -34,6 +34,7 @@ #include #include #include +#include "errlst.h" #include "printf.h" int @@ -54,7 +55,7 @@ ret = 0; error = *((const int *)arg[0]); - if (error >= 0 && error < sys_nerr) { + if (error >= 0 && error < __hidden_sys_nerr) { p = strerror(error); return (__printf_out(io, pi, p, strlen(p))); } Index: lib/libc/string/strerror.c =================================================================== --- lib/libc/string/strerror.c (revision 254670) +++ lib/libc/string/strerror.c (working copy) @@ -42,6 +42,8 @@ #include #include +#include "errlst.h" + #define UPREFIX "Unknown error" /* @@ -87,7 +89,7 @@ catd = catopen("libc", NL_CAT_LOCALE); #endif - if (errnum < 0 || errnum >= sys_nerr) { + if (errnum < 0 || errnum >= __hidden_sys_nerr) { errstr(errnum, #if defined(NLS) catgets(catd, 1, 0xffff, UPREFIX), @@ -99,9 +101,9 @@ } else { if (strlcpy(strerrbuf, #if defined(NLS) - catgets(catd, 1, errnum, sys_errlist[errnum]), + catgets(catd, 1, errnum, __hidden_sys_errlist[errnum]), #else - sys_errlist[errnum], + __hidden_sys_errlist[errnum], #endif buflen) >= buflen) retval = ERANGE; -- Jilles Tjoelker