From patchwork Tue Nov 15 15:55:08 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zack Weinberg X-Patchwork-Id: 17484 Received: (qmail 118655 invoked by alias); 15 Nov 2016 15:55:16 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 117975 invoked by uid 89); 15 Nov 2016 15:55:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.5 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=H*r:166.84.1, H*r:sk:mailbac, H*m:panix, __c X-HELO: mailbackend.panix.com From: Zack Weinberg To: libc-alpha@sourceware.org Cc: carlos@redhat.com, fweimer@redhat.com Subject: [PATCH 2/3] Add fortification and inline optimization of explicit_bzero. Date: Tue, 15 Nov 2016 10:55:08 -0500 Message-Id: <20161115155509.12692-3-zackw@panix.com> In-Reply-To: <20161115155509.12692-2-zackw@panix.com> References: <20161115155509.12692-1-zackw@panix.com> <20161115155509.12692-2-zackw@panix.com> MIME-Version: 1.0 By exposing __glibc_read_memory to external callers, we can write a fortify wrapper for explicit_bzero in terms of __memset_chk and __glibc_read_memory. There is no out-of-line __bzero_chk defined in libc.so, so I see no need to add an out-of-line __explicit_bzero_chk either. We can also add a bits/string2.h macro that optimizes explicit_bzero to memset + __glibc_read_memory, thus allowing the compiler to use intrinsic code generation for memset. I think this is worth doing because it partially addresses the problem pointed out in the manual, where explicit_bzero might _cause_ sensitive data to be copied out of registers onto the stack. With memset visible to the compiler, it will just clear a scratch area on the stack and then call __glibc_read_memory; it won't copy the sensitive data onto the stack first. (But it still doesn't erase the sensitive data _in_ the registers, so the warning in the manual is still appropriate.) * string/bits/string2.h: Optimize explicit_bzero. * string/bits/string3.h: Fortify explicit_bzero. * debug/tst-chk1.c: Test fortification of explicit_bzero. --- debug/tst-chk1.c | 28 ++++++++++++++++++++++++++++ string/bits/string2.h | 11 +++++++++++ string/bits/string3.h | 8 ++++++++ 3 files changed, 47 insertions(+) diff --git a/debug/tst-chk1.c b/debug/tst-chk1.c index 478c2fb..e87a279 100644 --- a/debug/tst-chk1.c +++ b/debug/tst-chk1.c @@ -160,6 +160,10 @@ do_test (void) if (memcmp (buf, "aabcdabc\0\0", 10)) FAIL (); + explicit_bzero (buf + 6, 4); + if (memcmp (buf, "aabcda\0\0\0\0", 10)) + FAIL (); + strcpy (buf + 4, "EDCBA"); if (memcmp (buf, "aabcEDCBA", 10)) FAIL (); @@ -201,6 +205,10 @@ do_test (void) if (memcmp (buf, "aabcdabc\0\0", 10)) FAIL (); + explicit_bzero (buf + 6, l0 + 4); + if (memcmp (buf, "aabcda\0\0\0\0", 10)) + FAIL (); + strcpy (buf + 4, str1 + 5); if (memcmp (buf, "aabcEDCBA", 10)) FAIL (); @@ -256,6 +264,10 @@ do_test (void) if (memcmp (a.buf1, "aabcdabc\0\0", 10)) FAIL (); + explicit_bzero (a.buf1 + 6, l0 + 4); + if (memcmp (a.buf1, "aabcda\0\0\0\0", 10)) + FAIL (); + #if __USE_FORTIFY_LEVEL < 2 /* The following tests are supposed to crash with -D_FORTIFY_SOURCE=2 and sufficient GCC support, as the string operations overflow @@ -345,6 +357,14 @@ do_test (void) bzero (buf + 9, l0 + 2); CHK_FAIL_END + CHK_FAIL_START + explicit_bzero (buf + 9, 2); + CHK_FAIL_END + + CHK_FAIL_START + explicit_bzero (buf + 9, l0 + 2); + CHK_FAIL_END + CHK_FAIL_START strcpy (buf + 5, str1 + 5); CHK_FAIL_END @@ -454,6 +474,14 @@ do_test (void) bzero (a.buf1 + 9, l0 + 2); CHK_FAIL_END + CHK_FAIL_START + explicit_bzero (a.buf1 + 9, 2); + CHK_FAIL_END + + CHK_FAIL_START + explicit_bzero (a.buf1 + 9, l0 + 2); + CHK_FAIL_END + # if __USE_FORTIFY_LEVEL >= 2 # define O 0 # else diff --git a/string/bits/string2.h b/string/bits/string2.h index ca1eda9..d509533 100644 --- a/string/bits/string2.h +++ b/string/bits/string2.h @@ -57,6 +57,17 @@ # define __bzero(s, n) __builtin_memset (s, '\0', n) #endif +#if defined __USE_MISC +/* As bzero, but the compiler will not delete a call to this function, + even if S is dead after the call. This is a macro instead of an + inline function _solely_ so that it will not get turned into an + external definition in string-inlines.o; it has its own .c file in + libc already. */ +# define explicit_bzero(s, n) \ + (__extension__ ({ void *__s = (s); size_t __n = (n); \ + memset (__s, '\0', __n); \ + __glibc_read_memory (__s, __n); })) +#endif #ifndef _HAVE_STRING_ARCH_strchr extern void *__rawmemchr (const void *__s, int __c); diff --git a/string/bits/string3.h b/string/bits/string3.h index 8f13b65..c9e2e62 100644 --- a/string/bits/string3.h +++ b/string/bits/string3.h @@ -42,6 +42,7 @@ __warndecl (__warn_memset_zero_len, # ifdef __USE_MISC # undef bcopy # undef bzero +# undef explicit_bzero # endif #endif @@ -102,6 +103,13 @@ __NTH (bzero (void *__dest, size_t __len)) { (void) __builtin___memset_chk (__dest, '\0', __len, __bos0 (__dest)); } + +__fortify_function void +__NTH (explicit_bzero (void *__dest, size_t __len)) +{ + (void) __builtin___memset_chk (__dest, '\0', __len, __bos0 (__dest)); + __glibc_read_memory (__dest, __len); +} #endif __fortify_function char *