* [PATCH 1/4] Add tests for fortification of bcopy and bzero.
2016-08-17 17:19 [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
@ 2016-08-17 17:19 ` Zack Weinberg
2016-08-19 12:50 ` Adhemerval Zanella
2016-08-17 17:19 ` [PATCH 3/4] Add fortification support for explicit_bzero Zack Weinberg
` (5 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Zack Weinberg @ 2016-08-17 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: fweimer
This was formerly bundled with the tests for fortification of
explicit_bzero, but is unconnected and I'd like to go ahead and land
it ASAP.
zw
* debug/tst-chk1.c: Add tests for fortification of bcopy and bzero.
---
debug/tst-chk1.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/debug/tst-chk1.c b/debug/tst-chk1.c
index 4f968ee..478c2fb 100644
--- a/debug/tst-chk1.c
+++ b/debug/tst-chk1.c
@@ -143,6 +143,11 @@ do_test (void)
if (memcmp (buf, "aabcdefghi", 10))
FAIL ();
+ memcpy (buf, "abcdefghij", 10);
+ bcopy (buf, buf + 1, 9);
+ if (memcmp (buf, "aabcdefghi", 10))
+ FAIL ();
+
if (mempcpy (buf + 5, "abcde", 5) != buf + 10
|| memcmp (buf, "aabcdabcde", 10))
FAIL ();
@@ -151,6 +156,10 @@ do_test (void)
if (memcmp (buf, "aabcdabcjj", 10))
FAIL ();
+ bzero (buf + 8, 2);
+ if (memcmp (buf, "aabcdabc\0\0", 10))
+ FAIL ();
+
strcpy (buf + 4, "EDCBA");
if (memcmp (buf, "aabcEDCBA", 10))
FAIL ();
@@ -175,6 +184,11 @@ do_test (void)
if (memcmp (buf, "aabcdefghi", 10))
FAIL ();
+ memcpy (buf, "abcdefghij", l0 + 10);
+ bcopy (buf, buf + 1, l0 + 9);
+ if (memcmp (buf, "aabcdefghi", 10))
+ FAIL ();
+
if (mempcpy (buf + 5, "abcde", l0 + 5) != buf + 10
|| memcmp (buf, "aabcdabcde", 10))
FAIL ();
@@ -183,6 +197,10 @@ do_test (void)
if (memcmp (buf, "aabcdabcjj", 10))
FAIL ();
+ bzero (buf + 8, l0 + 2);
+ if (memcmp (buf, "aabcdabc\0\0", 10))
+ FAIL ();
+
strcpy (buf + 4, str1 + 5);
if (memcmp (buf, "aabcEDCBA", 10))
FAIL ();
@@ -214,11 +232,18 @@ do_test (void)
if (memcmp (buf, "aabcEcdZY", 10))
FAIL ();
+ /* The following tests are supposed to succeed at all fortify
+ levels, even though they overflow a.buf1 into a.buf2. */
memcpy (a.buf1, "abcdefghij", l0 + 10);
memmove (a.buf1 + 1, a.buf1, l0 + 9);
if (memcmp (a.buf1, "aabcdefghi", 10))
FAIL ();
+ memcpy (a.buf1, "abcdefghij", l0 + 10);
+ bcopy (a.buf1, a.buf1 + 1, l0 + 9);
+ if (memcmp (a.buf1, "aabcdefghi", 10))
+ FAIL ();
+
if (mempcpy (a.buf1 + 5, "abcde", l0 + 5) != a.buf1 + 10
|| memcmp (a.buf1, "aabcdabcde", 10))
FAIL ();
@@ -227,6 +252,10 @@ do_test (void)
if (memcmp (a.buf1, "aabcdabcjj", 10))
FAIL ();
+ bzero (a.buf1 + 8, l0 + 2);
+ if (memcmp (a.buf1, "aabcdabc\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
@@ -284,6 +313,14 @@ do_test (void)
memmove (buf + 2, buf + 1, l0 + 9);
CHK_FAIL_END
+ CHK_FAIL_START
+ bcopy (buf + 1, buf + 2, 9);
+ CHK_FAIL_END
+
+ CHK_FAIL_START
+ bcopy (buf + 1, buf + 2, l0 + 9);
+ CHK_FAIL_END
+
CHK_FAIL_START
p = (char *) mempcpy (buf + 6, "abcde", 5);
CHK_FAIL_END
@@ -300,6 +337,14 @@ do_test (void)
memset (buf + 9, 'j', l0 + 2);
CHK_FAIL_END
+ CHK_FAIL_START
+ bzero (buf + 9, 2);
+ CHK_FAIL_END
+
+ CHK_FAIL_START
+ bzero (buf + 9, l0 + 2);
+ CHK_FAIL_END
+
CHK_FAIL_START
strcpy (buf + 5, str1 + 5);
CHK_FAIL_END
@@ -377,6 +422,14 @@ do_test (void)
memmove (a.buf1 + 2, a.buf1 + 1, l0 + 9);
CHK_FAIL_END
+ CHK_FAIL_START
+ bcopy (a.buf1 + 1, a.buf1 + 2, 9);
+ CHK_FAIL_END
+
+ CHK_FAIL_START
+ bcopy (a.buf1 + 1, a.buf1 + 2, l0 + 9);
+ CHK_FAIL_END
+
CHK_FAIL_START
p = (char *) mempcpy (a.buf1 + 6, "abcde", 5);
CHK_FAIL_END
@@ -393,6 +446,14 @@ do_test (void)
memset (a.buf1 + 9, 'j', l0 + 2);
CHK_FAIL_END
+ CHK_FAIL_START
+ bzero (a.buf1 + 9, 2);
+ CHK_FAIL_END
+
+ CHK_FAIL_START
+ bzero (a.buf1 + 9, l0 + 2);
+ CHK_FAIL_END
+
# if __USE_FORTIFY_LEVEL >= 2
# define O 0
# else
--
2.9.3
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/4] Add tests for fortification of bcopy and bzero.
2016-08-17 17:19 ` [PATCH 1/4] Add tests for fortification of bcopy and bzero Zack Weinberg
@ 2016-08-19 12:50 ` Adhemerval Zanella
2016-08-19 13:08 ` Zack Weinberg
0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2016-08-19 12:50 UTC (permalink / raw)
To: libc-alpha
LGTM.
On 17/08/2016 14:19, Zack Weinberg wrote:
> This was formerly bundled with the tests for fortification of
> explicit_bzero, but is unconnected and I'd like to go ahead and land
> it ASAP.
>
> zw
>
> * debug/tst-chk1.c: Add tests for fortification of bcopy and bzero.
> ---
> debug/tst-chk1.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/debug/tst-chk1.c b/debug/tst-chk1.c
> index 4f968ee..478c2fb 100644
> --- a/debug/tst-chk1.c
> +++ b/debug/tst-chk1.c
> @@ -143,6 +143,11 @@ do_test (void)
> if (memcmp (buf, "aabcdefghi", 10))
> FAIL ();
>
> + memcpy (buf, "abcdefghij", 10);
> + bcopy (buf, buf + 1, 9);
> + if (memcmp (buf, "aabcdefghi", 10))
> + FAIL ();
> +
> if (mempcpy (buf + 5, "abcde", 5) != buf + 10
> || memcmp (buf, "aabcdabcde", 10))
> FAIL ();
> @@ -151,6 +156,10 @@ do_test (void)
> if (memcmp (buf, "aabcdabcjj", 10))
> FAIL ();
>
> + bzero (buf + 8, 2);
> + if (memcmp (buf, "aabcdabc\0\0", 10))
> + FAIL ();
> +
> strcpy (buf + 4, "EDCBA");
> if (memcmp (buf, "aabcEDCBA", 10))
> FAIL ();
> @@ -175,6 +184,11 @@ do_test (void)
> if (memcmp (buf, "aabcdefghi", 10))
> FAIL ();
>
> + memcpy (buf, "abcdefghij", l0 + 10);
> + bcopy (buf, buf + 1, l0 + 9);
> + if (memcmp (buf, "aabcdefghi", 10))
> + FAIL ();
> +
> if (mempcpy (buf + 5, "abcde", l0 + 5) != buf + 10
> || memcmp (buf, "aabcdabcde", 10))
> FAIL ();
> @@ -183,6 +197,10 @@ do_test (void)
> if (memcmp (buf, "aabcdabcjj", 10))
> FAIL ();
>
> + bzero (buf + 8, l0 + 2);
> + if (memcmp (buf, "aabcdabc\0\0", 10))
> + FAIL ();
> +
> strcpy (buf + 4, str1 + 5);
> if (memcmp (buf, "aabcEDCBA", 10))
> FAIL ();
> @@ -214,11 +232,18 @@ do_test (void)
> if (memcmp (buf, "aabcEcdZY", 10))
> FAIL ();
>
> + /* The following tests are supposed to succeed at all fortify
> + levels, even though they overflow a.buf1 into a.buf2. */
> memcpy (a.buf1, "abcdefghij", l0 + 10);
> memmove (a.buf1 + 1, a.buf1, l0 + 9);
> if (memcmp (a.buf1, "aabcdefghi", 10))
> FAIL ();
>
> + memcpy (a.buf1, "abcdefghij", l0 + 10);
> + bcopy (a.buf1, a.buf1 + 1, l0 + 9);
> + if (memcmp (a.buf1, "aabcdefghi", 10))
> + FAIL ();
> +
> if (mempcpy (a.buf1 + 5, "abcde", l0 + 5) != a.buf1 + 10
> || memcmp (a.buf1, "aabcdabcde", 10))
> FAIL ();
> @@ -227,6 +252,10 @@ do_test (void)
> if (memcmp (a.buf1, "aabcdabcjj", 10))
> FAIL ();
>
> + bzero (a.buf1 + 8, l0 + 2);
> + if (memcmp (a.buf1, "aabcdabc\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
> @@ -284,6 +313,14 @@ do_test (void)
> memmove (buf + 2, buf + 1, l0 + 9);
> CHK_FAIL_END
>
> + CHK_FAIL_START
> + bcopy (buf + 1, buf + 2, 9);
> + CHK_FAIL_END
> +
> + CHK_FAIL_START
> + bcopy (buf + 1, buf + 2, l0 + 9);
> + CHK_FAIL_END
> +
> CHK_FAIL_START
> p = (char *) mempcpy (buf + 6, "abcde", 5);
> CHK_FAIL_END
> @@ -300,6 +337,14 @@ do_test (void)
> memset (buf + 9, 'j', l0 + 2);
> CHK_FAIL_END
>
> + CHK_FAIL_START
> + bzero (buf + 9, 2);
> + CHK_FAIL_END
> +
> + CHK_FAIL_START
> + bzero (buf + 9, l0 + 2);
> + CHK_FAIL_END
> +
> CHK_FAIL_START
> strcpy (buf + 5, str1 + 5);
> CHK_FAIL_END
> @@ -377,6 +422,14 @@ do_test (void)
> memmove (a.buf1 + 2, a.buf1 + 1, l0 + 9);
> CHK_FAIL_END
>
> + CHK_FAIL_START
> + bcopy (a.buf1 + 1, a.buf1 + 2, 9);
> + CHK_FAIL_END
> +
> + CHK_FAIL_START
> + bcopy (a.buf1 + 1, a.buf1 + 2, l0 + 9);
> + CHK_FAIL_END
> +
> CHK_FAIL_START
> p = (char *) mempcpy (a.buf1 + 6, "abcde", 5);
> CHK_FAIL_END
> @@ -393,6 +446,14 @@ do_test (void)
> memset (a.buf1 + 9, 'j', l0 + 2);
> CHK_FAIL_END
>
> + CHK_FAIL_START
> + bzero (a.buf1 + 9, 2);
> + CHK_FAIL_END
> +
> + CHK_FAIL_START
> + bzero (a.buf1 + 9, l0 + 2);
> + CHK_FAIL_END
> +
> # if __USE_FORTIFY_LEVEL >= 2
> # define O 0
> # else
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/4] Add fortification support for explicit_bzero.
2016-08-17 17:19 [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
2016-08-17 17:19 ` [PATCH 1/4] Add tests for fortification of bcopy and bzero Zack Weinberg
@ 2016-08-17 17:19 ` Zack Weinberg
2016-08-17 17:19 ` [PATCH 2/4] New string function explicit_bzero (from OpenBSD) Zack Weinberg
` (4 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Zack Weinberg @ 2016-08-17 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: fweimer
The __glibc_read_memory approach means that this is very easy - it can
be defined in terms of __memset_chk, which was not possible in the previous
iteration.
* string/bits/string3.h: Fortify explicit_bzero.
* string/bits/string2.h: Cooperate with this.
* debug/tst-chk1.c: Test fortification of explicit_bzero.
---
debug/tst-chk1.c | 28 ++++++++++++++++++++++++++++
string/bits/string2.h | 2 +-
string/bits/string3.h | 7 +++++++
3 files changed, 36 insertions(+), 1 deletion(-)
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 f890585..f7fc866 100644
--- a/string/bits/string2.h
+++ b/string/bits/string2.h
@@ -57,7 +57,7 @@
# define __bzero(s, n) __builtin_memset (s, '\0', n)
#endif
-#ifdef __USE_MISC
+#if defined __USE_MISC && !defined __fortify_function
/* As bzero, but the compiler will not delete a call to this
function, even if S is dead after the call. Note: this function
has its own implementation file and should not be slurped into
diff --git a/string/bits/string3.h b/string/bits/string3.h
index dd8db68..d340bef 100644
--- a/string/bits/string3.h
+++ b/string/bits/string3.h
@@ -102,6 +102,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 *
--
2.9.3
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-17 17:19 [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
2016-08-17 17:19 ` [PATCH 1/4] Add tests for fortification of bcopy and bzero Zack Weinberg
2016-08-17 17:19 ` [PATCH 3/4] Add fortification support for explicit_bzero Zack Weinberg
@ 2016-08-17 17:19 ` Zack Weinberg
2016-08-18 18:31 ` Florian Weimer
2016-08-17 17:19 ` [PATCH 4/4] Use explicit_bzero where appropriate Zack Weinberg
` (3 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Zack Weinberg @ 2016-08-17 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: fweimer
explicit_bzero(s, n) is the same as memset(s, 0, n), except that the
compiler is not allowed to delete a call to explicit_bzero even if the
memory pointed to by 's' is dead after the call. We achieve this effect
by defining it to call memset() and then a second function,
extern void __glibc_read_memory (const void *, size_t)
__attribute_noinline__;
which does nothing -- but the compiler is prevented from knowing that
it does nothing, and so the pointer "escapes" and the memory is not
treated as dead. (Concretely, __glibc_read_memory is forced
out-of-line, defined in a file containing nothing else, and comments
in both string/read_memory.c and string/Makefile document that it must
not be subject to link-time optimization.)
* string/explicit_bzero.c, string/read_memory.c: New routines.
* string/test-explicit_bzero.c: New test.
* string/Makefile (routines, strop-tests): Add them.
* string/test-memset.c: Add ifdeffage for testing explicit_bzero.
* string/string.h [__USE_MISC]: Declare explicit_bzero and
__glibc_read_memory.
* string/bits/string2.h [__USE_MISC]: Provide inline version of
explicit_bzero.
* manual/string.texi: Document explicit_bzero.
* string/Versions [GLIBC_2.25]: Add explicit_bzero and
__glibc_read_memory.
* include/string.h: Hidden prototype for __internal_glibc_read_memory.
* sysdeps/arm/nacl/libc.abilist
* sysdeps/unix/sysv/linux/aarch64/libc.abilist
* sysdeps/unix/sysv/linux/alpha/libc.abilist
* sysdeps/unix/sysv/linux/arm/libc.abilist
* sysdeps/unix/sysv/linux/hppa/libc.abilist
* sysdeps/unix/sysv/linux/i386/libc.abilist
* sysdeps/unix/sysv/linux/ia64/libc.abilist
* sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
* sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
* sysdeps/unix/sysv/linux/microblaze/libc.abilist
* sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
* sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
* sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
* sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
* sysdeps/unix/sysv/linux/nios2/libc.abilist
* sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
* sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
* sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist
* sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist
* sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
* sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
* sysdeps/unix/sysv/linux/sh/libc.abilist
* sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
* sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
* sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist
* sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist
* sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist
* sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
* sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist:
Add entries for explicit_bzero and __glibc_read_memory.
---
include/string.h | 2 +
manual/string.texi | 101 +++++++++++++++++++++
string/Makefile | 10 +-
string/Versions | 7 ++
string/bits/string2.h | 14 ++-
string/explicit_bzero.c | 30 ++++++
string/read_memory.c | 41 +++++++++
string/string.h | 9 ++
string/test-explicit_bzero.c | 20 ++++
string/test-memset.c | 10 +-
sysdeps/arm/nacl/libc.abilist | 2 +
sysdeps/unix/sysv/linux/aarch64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/alpha/libc.abilist | 2 +
sysdeps/unix/sysv/linux/arm/libc.abilist | 2 +
sysdeps/unix/sysv/linux/hppa/libc.abilist | 2 +
sysdeps/unix/sysv/linux/i386/libc.abilist | 2 +
sysdeps/unix/sysv/linux/ia64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist | 2 +
sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist | 2 +
sysdeps/unix/sysv/linux/microblaze/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips32/fpu/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips32/nofpu/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips64/n32/libc.abilist | 2 +
.../unix/sysv/linux/mips/mips64/n64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/nios2/libc.abilist | 2 +
.../sysv/linux/powerpc/powerpc32/fpu/libc.abilist | 2 +
.../linux/powerpc/powerpc32/nofpu/libc.abilist | 2 +
.../sysv/linux/powerpc/powerpc64/libc-le.abilist | 2 +
.../unix/sysv/linux/powerpc/powerpc64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist | 2 +
sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/sh/libc.abilist | 2 +
sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist | 2 +
sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist | 2 +
.../sysv/linux/tile/tilegx/tilegx32/libc.abilist | 2 +
.../sysv/linux/tile/tilegx/tilegx64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist | 2 +
sysdeps/unix/sysv/linux/x86_64/64/libc.abilist | 2 +
sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist | 2 +
39 files changed, 298 insertions(+), 4 deletions(-)
create mode 100644 string/explicit_bzero.c
create mode 100644 string/read_memory.c
create mode 100644 string/test-explicit_bzero.c
diff --git a/include/string.h b/include/string.h
index e145bfd..277c07d 100644
--- a/include/string.h
+++ b/include/string.h
@@ -99,6 +99,8 @@ libc_hidden_proto (memmem)
extern __typeof (memmem) __memmem;
libc_hidden_proto (__memmem)
libc_hidden_proto (__ffs)
+extern __typeof (__glibc_read_memory) __internal_glibc_read_memory;
+libc_hidden_proto (__internal_glibc_read_memory)
libc_hidden_builtin_proto (memchr)
libc_hidden_builtin_proto (memcpy)
diff --git a/manual/string.texi b/manual/string.texi
index bce81a7..fe4ca48 100644
--- a/manual/string.texi
+++ b/manual/string.texi
@@ -34,6 +34,8 @@ too.
* Search Functions:: Searching for a specific element or substring.
* Finding Tokens in a String:: Splitting a string into tokens by looking
for delimiters.
+* Erasing Sensitive Data:: Clearing memory which contains sensitive
+ data, after it's no longer needed.
* strfry:: Function for flash-cooking a string.
* Trivial Encryption:: Obscuring data.
* Encode Binary Data:: Encoding and Decoding of Binary Data.
@@ -2375,6 +2377,105 @@ contains no '/' bytes, then "." is returned. The prototype for this
function can be found in @file{libgen.h}.
@end deftypefun
+@node Erasing Sensitive Data
+@section Erasing Sensitive Data
+
+It is sometimes necessary to make sure that a block of data in memory
+is erased after use, even if no correct C program could access it
+again. For instance, a cryptographic key should not be allowed to
+survive on the stack after the program is finished using it, because
+there might be a bug that causes junk stack data, including the key,
+to be revealed to the outside world. @code{memset} and @code{bzero}
+are not safe to use for this, because the C compiler knows what they
+do, and can delete ``unnecessary'' calls to them. For this situation,
+@theglibc{} provides @code{explicit_bzero}, which is functionally
+identical to @code{bzero}, except that the C compiler will @emph{not}
+delete apparently-unnecessary calls.
+
+@comment string.h
+@comment BSD
+@deftypefun void explicit_bzero (void *@var{block}, size_t @var{len})
+@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
+
+@code{explicit_bzero} writes zero into each of the first @var{len}
+bytes of the object beginning at @var{block}, just as @code{bzero}
+would. The compiler will not delete a call to this function, even
+if the object beginning at @var{block} is never referred to again.
+
+@smallexample
+#include <string.h>
+
+extern void encrypt (const char *key, const char *in,
+ char *out, size_t n);
+extern void genkey (const char *phrase, char *key);
+
+void encrypt_with_phrase (const char *phrase, const char *in,
+ char *out, size_t n)
+@{
+ char key[16];
+ genkey (phrase, key);
+ encrypt (key, in, out, n);
+ explicit_bzero (key, 16);
+@}
+@end smallexample
+
+@noindent
+If @code{bzero} or @code{memset} had been used in this function, the C
+compiler might remove it as unnecessary, but it will not do this with
+@code{explicit_bzero}.
+
+@strong{Warning:} The @emph{only} optimization disabled by
+@code{explicit_bzero} is removal of ``unnecessary'' calls. In all
+other respects, the compiler is allowed to optimize as it would for
+@code{memset}. For instance, it may deduce that @var{block} cannot be
+a null pointer, and propagate this information both forward and
+backward in control flow.
+
+@strong{Warning:} The compiler is free to make additional copies of
+any object, or parts of it, in temporary storage areas (such as
+registers and ``scratch'' stack space). @code{explicit_bzero} does
+not guarantee that temporary copies of sensitive data are destroyed.
+In fact, in some situations, using @code{explicit_bzero} will
+@emph{cause} creation of an additional copy, and only that copy will
+be cleared:
+
+@smallexample
+#include <string.h>
+
+struct key @{
+ unsigned long long low;
+ unsigned long long high;
+@};
+
+struct key get_key(void);
+void use_key(struct key);
+
+void
+with_clear(void)
+@{
+ struct key k;
+ k = get_key();
+ use_key(k);
+ explicit_bzero(&k, sizeof(k));
+@}
+@end smallexample
+
+@noindent
+Without the call to @code{explicit_bzero}, @var{k} might not need to
+be stored in memory: depending on the ABI, its value could be returned
+from @code{get_key} and passed to @code{use_key} using only CPU
+registers. @code{explicit_bzero} operates on memory, so the compiler
+has to make a copy of @var{k} in memory for it, and the copy in the
+CPU registers remains intact. This can occur for any variable whose
+address is only taken in a call to @code{explicit_bzero}, even if it
+might seem ``too large'' to be stored in registers.
+
+@strong{Portability Note:} This function first appeared in OpenBSD 5.5
+and has not been standardized. @Theglibc{} declares this function in
+@file{string.h}, but on other systems it may be in @file{strings.h}
+instead.
+@end deftypefun
+
@node strfry
@section strfry
diff --git a/string/Makefile b/string/Makefile
index 9c87419..180ec86 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -41,13 +41,19 @@ routines := strcat strchr strcmp strcoll strcpy strcspn \
addsep replace) \
envz basename \
strcoll_l strxfrm_l string-inlines memrchr \
- xpg-strerror strerror_l
+ xpg-strerror strerror_l explicit_bzero
+
+# Attention future hackers trying to enable link-time optimization for
+# glibc: this file *must not* be subject to LTO. It is added separately
+# to 'routines' to document this. See comments in this file for details.
+routines += read_memory
strop-tests := memchr memcmp memcpy memmove mempcpy memset memccpy \
stpcpy stpncpy strcat strchr strcmp strcpy strcspn \
strlen strncmp strncpy strpbrk strrchr strspn memmem \
strstr strcasestr strnlen strcasecmp strncasecmp \
- strncat rawmemchr strchrnul bcopy bzero memrchr
+ strncat rawmemchr strchrnul bcopy bzero memrchr \
+ explicit_bzero
tests := tester inl-tester noinl-tester testcopy test-ffs \
tst-strlen stratcliff tst-svc tst-inlcall \
bug-strncat1 bug-strspn1 bug-strpbrk1 tst-bswap \
diff --git a/string/Versions b/string/Versions
index 475c1fd..b5c24f4 100644
--- a/string/Versions
+++ b/string/Versions
@@ -82,4 +82,11 @@ libc {
}
GLIBC_2.24 {
}
+ GLIBC_2.25 {
+ # used by inlines in string2.h
+ __glibc_read_memory;
+
+ # e*
+ explicit_bzero;
+ }
}
diff --git a/string/bits/string2.h b/string/bits/string2.h
index 8098760..f890585 100644
--- a/string/bits/string2.h
+++ b/string/bits/string2.h
@@ -52,11 +52,23 @@
#define __string2_1bptr_p(__x) \
((size_t)(const void *)((__x) + 1) - (size_t)(const void *)(__x) == 1)
-/* Set N bytes of S to C. */
+/* Set N bytes of S to 0. */
#if !defined _HAVE_STRING_ARCH_memset
# define __bzero(s, n) __builtin_memset (s, '\0', n)
#endif
+#ifdef __USE_MISC
+/* As bzero, but the compiler will not delete a call to this
+ function, even if S is dead after the call. Note: this function
+ has its own implementation file and should not be slurped into
+ string-inlines.o. */
+__extern_inline void
+explicit_bzero (void *__s, size_t __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/explicit_bzero.c b/string/explicit_bzero.c
new file mode 100644
index 0000000..a02be4f
--- /dev/null
+++ b/string/explicit_bzero.c
@@ -0,0 +1,30 @@
+/* Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <features.h>
+#undef __USE_STRING_INLINES
+#define __NO_STRING_INLINES
+#include <string.h>
+
+/* Set LEN bytes of S to 0. The compiler will not delete a call to
+ this function, even if S is dead after the call. */
+void
+explicit_bzero (void *s, size_t len)
+{
+ memset (s, '\0', len);
+ __internal_glibc_read_memory (s, len);
+}
diff --git a/string/read_memory.c b/string/read_memory.c
new file mode 100644
index 0000000..c4a5990
--- /dev/null
+++ b/string/read_memory.c
@@ -0,0 +1,41 @@
+/* Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <string.h>
+
+/* This function is an optimization fence. It doesn't do anything
+ itself, but calls to it prevent calls to explicit_bzero from being
+ optimized away. In order to achieve this effect, this function
+ must never, under any circumstances, be inlined or subjected to
+ inter-procedural optimization. string.h declares this function
+ with attributes that, in conjunction with the no-op asm insert, are
+ sufficient to prevent problems in the current (2016) generation of
+ compilers, but *only if* this file is *not* compiled with -flto.
+ At present, this is not an issue since glibc is never compiled with
+ -flto, but should that ever change, this file must be excepted.
+
+ The 'volatile' below is technically not necessary but is included
+ for explicitness. */
+
+void
+internal_function
+__internal_glibc_read_memory(const void *s, size_t len)
+{
+ asm volatile ("");
+}
+libc_hidden_def (__internal_glibc_read_memory)
+strong_alias (__internal_glibc_read_memory, __glibc_read_memory)
diff --git a/string/string.h b/string/string.h
index 57deaa4..40851b5 100644
--- a/string/string.h
+++ b/string/string.h
@@ -455,6 +455,15 @@ extern void bcopy (const void *__src, void *__dest, size_t __n)
/* Set N bytes of S to 0. */
extern void bzero (void *__s, size_t __n) __THROW __nonnull ((1));
+/* As bzero, but the compiler will not delete a call to this
+ function, even if S is dead after the call. */
+extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1));
+
+/* Optimization fence, used by bits/string2.h inline version of
+ explicit_bzero. */
+extern void __glibc_read_memory (const void *__s, size_t __n)
+ __THROW __nonnull ((1)) __attribute_noinline__;
+
/* Compare N bytes of S1 and S2 (same as memcmp). */
extern int bcmp (const void *__s1, const void *__s2, size_t __n)
__THROW __attribute_pure__ __nonnull ((1, 2));
diff --git a/string/test-explicit_bzero.c b/string/test-explicit_bzero.c
new file mode 100644
index 0000000..5a4543b
--- /dev/null
+++ b/string/test-explicit_bzero.c
@@ -0,0 +1,20 @@
+/* Test and measure explicit_bzero.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+#define TEST_EXPLICIT_BZERO
+#define TEST_BZERO
+#include "test-memset.c"
diff --git a/string/test-memset.c b/string/test-memset.c
index fee3bdf..7ca4f20 100644
--- a/string/test-memset.c
+++ b/string/test-memset.c
@@ -19,7 +19,11 @@
#define TEST_MAIN
#ifdef TEST_BZERO
-# define TEST_NAME "bzero"
+# ifdef TEST_EXPLICIT_BZERO
+# define TEST_NAME "explicit_bzero"
+# else
+# define TEST_NAME "bzero"
+# endif
#else
# ifndef WIDE
# define TEST_NAME "memset"
@@ -56,7 +60,11 @@ void builtin_bzero (char *, size_t);
IMPL (simple_bzero, 0)
IMPL (builtin_bzero, 0)
+#ifdef TEST_EXPLICIT_BZERO
+IMPL (explicit_bzero, 1)
+#else
IMPL (bzero, 1)
+#endif
void
simple_bzero (char *s, size_t n)
diff --git a/sysdeps/arm/nacl/libc.abilist b/sysdeps/arm/nacl/libc.abilist
index 4d3b0b9..02d4bc6 100644
--- a/sysdeps/arm/nacl/libc.abilist
+++ b/sysdeps/arm/nacl/libc.abilist
@@ -1843,6 +1843,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.25 gnu_dev_major F
GLIBC_2.25 gnu_dev_makedev F
GLIBC_2.25 gnu_dev_minor F
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc.abilist b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
index 2c2f49e..44dc0d1 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/aarch64/libc.abilist
@@ -2090,3 +2090,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
diff --git a/sysdeps/unix/sysv/linux/alpha/libc.abilist b/sysdeps/unix/sysv/linux/alpha/libc.abilist
index 8afba47..02a490d 100644
--- a/sysdeps/unix/sysv/linux/alpha/libc.abilist
+++ b/sysdeps/unix/sysv/linux/alpha/libc.abilist
@@ -2001,6 +2001,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/arm/libc.abilist b/sysdeps/unix/sysv/linux/arm/libc.abilist
index de3bdf4..d2b2800 100644
--- a/sysdeps/unix/sysv/linux/arm/libc.abilist
+++ b/sysdeps/unix/sysv/linux/arm/libc.abilist
@@ -91,6 +91,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.4 GLIBC_2.4 A
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0xa0
diff --git a/sysdeps/unix/sysv/linux/hppa/libc.abilist b/sysdeps/unix/sysv/linux/hppa/libc.abilist
index 3261b93..59b2591 100644
--- a/sysdeps/unix/sysv/linux/hppa/libc.abilist
+++ b/sysdeps/unix/sysv/linux/hppa/libc.abilist
@@ -1855,6 +1855,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/i386/libc.abilist b/sysdeps/unix/sysv/linux/i386/libc.abilist
index 6465a55..e64f9c6 100644
--- a/sysdeps/unix/sysv/linux/i386/libc.abilist
+++ b/sysdeps/unix/sysv/linux/i386/libc.abilist
@@ -2013,6 +2013,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/ia64/libc.abilist b/sysdeps/unix/sysv/linux/ia64/libc.abilist
index 4536271..78acf45 100644
--- a/sysdeps/unix/sysv/linux/ia64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/ia64/libc.abilist
@@ -1877,6 +1877,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
index 287d7a5..ceb1909 100644
--- a/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/coldfire/libc.abilist
@@ -92,6 +92,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.4 GLIBC_2.4 A
GLIBC_2.4 _Exit F
GLIBC_2.4 _IO_2_1_stderr_ D 0x98
diff --git a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
index c9229fa..e8d7256 100644
--- a/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
+++ b/sysdeps/unix/sysv/linux/m68k/m680x0/libc.abilist
@@ -1969,6 +1969,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/microblaze/libc.abilist b/sysdeps/unix/sysv/linux/microblaze/libc.abilist
index 0409791..c2e2ecc 100644
--- a/sysdeps/unix/sysv/linux/microblaze/libc.abilist
+++ b/sysdeps/unix/sysv/linux/microblaze/libc.abilist
@@ -2090,3 +2090,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
index f31653e..705f00b 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/fpu/libc.abilist
@@ -1944,6 +1944,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
index a56bd99..41d3e5c 100644
--- a/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips32/nofpu/libc.abilist
@@ -1942,6 +1942,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
index 44552df..4de5839 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n32/libc.abilist
@@ -1940,6 +1940,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
index 8d2a09d..e1d5bec 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/libc.abilist
@@ -1935,6 +1935,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/nios2/libc.abilist b/sysdeps/unix/sysv/linux/nios2/libc.abilist
index 0443b92..34e83a6 100644
--- a/sysdeps/unix/sysv/linux/nios2/libc.abilist
+++ b/sysdeps/unix/sysv/linux/nios2/libc.abilist
@@ -2131,3 +2131,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
index ba9a29a..91a30c8 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/fpu/libc.abilist
@@ -1973,6 +1973,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
index f19534c..a4ecbc2 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/nofpu/libc.abilist
@@ -1978,6 +1978,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist
index f8de1ab..277ec26 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc-le.abilist
@@ -2178,3 +2178,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist
index 6819133..e3dafa5 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/libc.abilist
@@ -92,6 +92,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 _Exit F
GLIBC_2.3 _IO_2_1_stderr_ D 0xe0
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
index 4cd5d85..3ab826d 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/libc.abilist
@@ -1973,6 +1973,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
index 8cdb9df..c4c0a8e 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist
@@ -1874,6 +1874,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/sh/libc.abilist b/sysdeps/unix/sysv/linux/sh/libc.abilist
index 69791b4..7a11b8a 100644
--- a/sysdeps/unix/sysv/linux/sh/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sh/libc.abilist
@@ -1859,6 +1859,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
index fce58a8..13efb9f 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/libc.abilist
@@ -1965,6 +1965,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
index 16ce739..da6819b 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/libc.abilist
@@ -1903,6 +1903,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist
index f99c298..0d58dc0 100644
--- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx32/libc.abilist
@@ -2097,3 +2097,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
diff --git a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist
index c601ab0..fc55930 100644
--- a/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilegx/tilegx64/libc.abilist
@@ -2097,3 +2097,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
diff --git a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist
index f99c298..0d58dc0 100644
--- a/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist
+++ b/sysdeps/unix/sysv/linux/tile/tilepro/libc.abilist
@@ -2097,3 +2097,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
diff --git a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
index 8e6fa57..858a26c 100644
--- a/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/64/libc.abilist
@@ -1854,6 +1854,8 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
GLIBC_2.3 GLIBC_2.3 A
GLIBC_2.3 __ctype_b_loc F
GLIBC_2.3 __ctype_tolower_loc F
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
index 1e12f48..9a45eed 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/libc.abilist
@@ -2097,3 +2097,5 @@ GLIBC_2.23 fts64_set F
GLIBC_2.24 GLIBC_2.24 A
GLIBC_2.24 quick_exit F
GLIBC_2.25 GLIBC_2.25 A
+GLIBC_2.25 __glibc_read_memory F
+GLIBC_2.25 explicit_bzero F
--
2.9.3
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-17 17:19 ` [PATCH 2/4] New string function explicit_bzero (from OpenBSD) Zack Weinberg
@ 2016-08-18 18:31 ` Florian Weimer
2016-08-18 20:53 ` Torvald Riegel
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer @ 2016-08-18 18:31 UTC (permalink / raw)
To: Zack Weinberg, libc-alpha
On 08/17/2016 07:19 PM, Zack Weinberg wrote:
> +#ifdef __USE_MISC
> +/* As bzero, but the compiler will not delete a call to this
> + function, even if S is dead after the call. Note: this function
> + has its own implementation file and should not be slurped into
> + string-inlines.o. */
> +__extern_inline void
> +explicit_bzero (void *__s, size_t __n)
> +{
> + memset (__s, '\0', __n);
> + __glibc_read_memory (__s, __n);
> +}
> +#endif
__extern_inline can expand to nothing at all, and you would get multiple
definitions of explicit_bzero this way.
I don't think we want explicit_bzero to be inlined, it's useful to have
this name in the executable. Furthermore, we might want to add
additional state clearing later, so an implementation in libc.so.6 seems
desirable anyway.
For an implementation in libc, there is currently no different between
the __glibc_read_memory kludge and a full memory barrier, so I suggest
to go with the latter. (The explicit_bzero call will serve as a rather
broad barrier anyway, but we can annotate it with __THROW.)
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-18 18:31 ` Florian Weimer
@ 2016-08-18 20:53 ` Torvald Riegel
2016-08-19 12:44 ` Adhemerval Zanella
2016-08-19 12:50 ` Florian Weimer
0 siblings, 2 replies; 27+ messages in thread
From: Torvald Riegel @ 2016-08-18 20:53 UTC (permalink / raw)
To: Florian Weimer; +Cc: Zack Weinberg, libc-alpha
On Thu, 2016-08-18 at 20:31 +0200, Florian Weimer wrote:
> On 08/17/2016 07:19 PM, Zack Weinberg wrote:
> > +#ifdef __USE_MISC
> > +/* As bzero, but the compiler will not delete a call to this
> > + function, even if S is dead after the call. Note: this function
> > + has its own implementation file and should not be slurped into
> > + string-inlines.o. */
> > +__extern_inline void
> > +explicit_bzero (void *__s, size_t __n)
> > +{
> > + memset (__s, '\0', __n);
> > + __glibc_read_memory (__s, __n);
> > +}
> > +#endif
>
> __extern_inline can expand to nothing at all, and you would get multiple
> definitions of explicit_bzero this way.
>
> I don't think we want explicit_bzero to be inlined, it's useful to have
> this name in the executable. Furthermore, we might want to add
> additional state clearing later, so an implementation in libc.so.6 seems
> desirable anyway.
>
> For an implementation in libc, there is currently no different between
> the __glibc_read_memory kludge and a full memory barrier, so I suggest
> to go with the latter. (The explicit_bzero call will serve as a rather
> broad barrier anyway, but we can annotate it with __THROW.)
I suppose we just want a compiler barrier here though, and don't need a
memory barrier in the sense of something that constrains HW reordering.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-18 20:53 ` Torvald Riegel
@ 2016-08-19 12:44 ` Adhemerval Zanella
2016-08-19 13:00 ` Zack Weinberg
2016-08-19 12:50 ` Florian Weimer
1 sibling, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2016-08-19 12:44 UTC (permalink / raw)
To: libc-alpha
On 18/08/2016 17:53, Torvald Riegel wrote:
> On Thu, 2016-08-18 at 20:31 +0200, Florian Weimer wrote:
>> On 08/17/2016 07:19 PM, Zack Weinberg wrote:
>>> +#ifdef __USE_MISC
>>> +/* As bzero, but the compiler will not delete a call to this
>>> + function, even if S is dead after the call. Note: this function
>>> + has its own implementation file and should not be slurped into
>>> + string-inlines.o. */
>>> +__extern_inline void
>>> +explicit_bzero (void *__s, size_t __n)
>>> +{
>>> + memset (__s, '\0', __n);
>>> + __glibc_read_memory (__s, __n);
>>> +}
>>> +#endif
>>
>> __extern_inline can expand to nothing at all, and you would get multiple
>> definitions of explicit_bzero this way.
>>
>> I don't think we want explicit_bzero to be inlined, it's useful to have
>> this name in the executable. Furthermore, we might want to add
>> additional state clearing later, so an implementation in libc.so.6 seems
>> desirable anyway.
>>
>> For an implementation in libc, there is currently no different between
>> the __glibc_read_memory kludge and a full memory barrier, so I suggest
>> to go with the latter. (The explicit_bzero call will serve as a rather
>> broad barrier anyway, but we can annotate it with __THROW.)
>
> I suppose we just want a compiler barrier here though, and don't need a
> memory barrier in the sense of something that constrains HW reordering.
I would also suggest to avoid adding this inline optimization, it just add
another exported symbol by glibc with little performance benefit.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-19 12:44 ` Adhemerval Zanella
@ 2016-08-19 13:00 ` Zack Weinberg
2016-08-19 14:01 ` Adhemerval Zanella
0 siblings, 1 reply; 27+ messages in thread
From: Zack Weinberg @ 2016-08-19 13:00 UTC (permalink / raw)
To: libc-alpha
On 08/19/2016 08:43 AM, Adhemerval Zanella wrote:
> On 18/08/2016 17:53, Torvald Riegel wrote:
>> On Thu, 2016-08-18 at 20:31 +0200, Florian Weimer wrote:
>>> On 08/17/2016 07:19 PM, Zack Weinberg wrote:
>>>> +#ifdef __USE_MISC
>>>> +/* As bzero, but the compiler will not delete a call to this
>>>> + function, even if S is dead after the call. Note: this function
>>>> + has its own implementation file and should not be slurped into
>>>> + string-inlines.o. */
>>>> +__extern_inline void
>>>> +explicit_bzero (void *__s, size_t __n)
>>>> +{
>>>> + memset (__s, '\0', __n);
>>>> + __glibc_read_memory (__s, __n);
>>>> +}
>>>> +#endif
>>>
>>> __extern_inline can expand to nothing at all, and you would get multiple
>>> definitions of explicit_bzero this way.
OK, what should I be using instead?
>>> I don't think we want explicit_bzero to be inlined, it's useful to have
>>> this name in the executable. Furthermore, we might want to add
>>> additional state clearing later, so an implementation in libc.so.6 seems
>>> desirable anyway.
Oddly enough, inlining the memset leads to better security properties
for generated code. Remember the "key that lives in the vector
registers until it has to be copied onto the stack to be erased" scenario?
---
#include <emmintrin.h>
#include <string.h>
extern void explicit_bzero (void *ptr, size_t size);
extern __m128 get_key ();
extern void encrypt_with_key (__m128 key, void *data, size_t size);
void encrypt (void *data, size_t size)
{
__m128 key = get_key ();
encrypt_with_key (key, data, size);
explicit_bzero (&key, sizeof key);
}
---
(Assume an ABI where __m128 actually gets passed in registers.)
If you expose the memset to inlining, the key *won't* be copied into
the stack and then immediately erased; the compiler will just dutifully
clear 16 bytes of the stack and then pass them to __glibc_read_memory.
It still won't be erased from the vector registers, but that can only be
fixed with compiler support.
(It also leads to significantly more compact code -- see my experiment
with OpenSSL from last year -- but that's not as important.)
>> I suppose we just want a compiler barrier here though, and don't need a
>> memory barrier in the sense of something that constrains HW reordering.
That is correct; however, the "memory barrier" I was talking about was
asm volatile ("" ::: "memory");
which is also only a compiler barrier.
> I would also suggest to avoid adding this inline optimization, it just add
> another exported symbol by glibc with little performance benefit.
The *other* reason why I want to keep the inline optimization is that it
means I don't need to add __explicit_bzero_chk (which, because of
ifuncs, means futzing with *every sysdeps definition of memset*!). So
either way there's going to be a new exported symbol.
zw
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-19 13:00 ` Zack Weinberg
@ 2016-08-19 14:01 ` Adhemerval Zanella
2016-08-19 16:06 ` Zack Weinberg
0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2016-08-19 14:01 UTC (permalink / raw)
To: libc-alpha
On 19/08/2016 10:00, Zack Weinberg wrote:
> On 08/19/2016 08:43 AM, Adhemerval Zanella wrote:
>> On 18/08/2016 17:53, Torvald Riegel wrote:
>>> On Thu, 2016-08-18 at 20:31 +0200, Florian Weimer wrote:
>>>> On 08/17/2016 07:19 PM, Zack Weinberg wrote:
>>>>> +#ifdef __USE_MISC
>>>>> +/* As bzero, but the compiler will not delete a call to this
>>>>> + function, even if S is dead after the call. Note: this function
>>>>> + has its own implementation file and should not be slurped into
>>>>> + string-inlines.o. */
>>>>> +__extern_inline void
>>>>> +explicit_bzero (void *__s, size_t __n)
>>>>> +{
>>>>> + memset (__s, '\0', __n);
>>>>> + __glibc_read_memory (__s, __n);
>>>>> +}
>>>>> +#endif
>>>>
>>>> __extern_inline can expand to nothing at all, and you would get multiple
>>>> definitions of explicit_bzero this way.
>
> OK, what should I be using instead?
Fortify functions uses '__fortify_function' which defines both
__extern_always_inline and __attribute_artificial__. However fortify is guarded
by _FORTIFY_SOURCE, which requires GCC 4.1 (so __extern_always_inline will be
defined anyway).
Stdio also defines:
#ifndef __extern_inline
# define __STDIO_INLINE inline
#else
# define __STDIO_INLINE __extern_inline
#endif
#ifdef __USE_EXTERN_INLINES
...
#endif
But I think this will also only add unnecessary complexity on the patch.
Also, another issue with gnu_inline is when building for C++ it might get also
multiple definitions [1].
That's why I think simpler solution would be just to remove this inline
optimization.
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=41194
>
>>>> I don't think we want explicit_bzero to be inlined, it's useful to have
>>>> this name in the executable. Furthermore, we might want to add
>>>> additional state clearing later, so an implementation in libc.so.6 seems
>>>> desirable anyway.
>
> Oddly enough, inlining the memset leads to better security properties
> for generated code. Remember the "key that lives in the vector
> registers until it has to be copied onto the stack to be erased" scenario?
>
> ---
> #include <emmintrin.h>
> #include <string.h>
>
> extern void explicit_bzero (void *ptr, size_t size);
>
> extern __m128 get_key ();
> extern void encrypt_with_key (__m128 key, void *data, size_t size);
>
> void encrypt (void *data, size_t size)
> {
> __m128 key = get_key ();
> encrypt_with_key (key, data, size);
> explicit_bzero (&key, sizeof key);
> }
> ---
>
> (Assume an ABI where __m128 actually gets passed in registers.)
>
> If you expose the memset to inlining, the key *won't* be copied into
> the stack and then immediately erased; the compiler will just dutifully
> clear 16 bytes of the stack and then pass them to __glibc_read_memory.
> It still won't be erased from the vector registers, but that can only be
> fixed with compiler support.
>
> (It also leads to significantly more compact code -- see my experiment
> with OpenSSL from last year -- but that's not as important.)
I also think compact code should not be a blocker here as well. However I
do not have a better solution for this specific solution without compiler
support.
>
>>> I suppose we just want a compiler barrier here though, and don't need a
>>> memory barrier in the sense of something that constrains HW reordering.
>
> That is correct; however, the "memory barrier" I was talking about was
>
> asm volatile ("" ::: "memory");
>
> which is also only a compiler barrier.
>
>> I would also suggest to avoid adding this inline optimization, it just add
>> another exported symbol by glibc with little performance benefit.
>
> The *other* reason why I want to keep the inline optimization is that it
> means I don't need to add __explicit_bzero_chk (which, because of
> ifuncs, means futzing with *every sysdeps definition of memset*!). So
> either way there's going to be a new exported symbol.
>
> zw
I do not think this is really required, you can still add a default
debug/explicit_bzero_chk.c which will call explicit_bzero/memset. Each
port already defines how internal memset is called (either using
internal ifunc or redirecting to a default symbol) so I see no need
to add arch specific implementations.
This does not prevent though to later we add arch-specific optimized
version (which might be ifunc).
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-19 14:01 ` Adhemerval Zanella
@ 2016-08-19 16:06 ` Zack Weinberg
2016-08-19 16:32 ` Adhemerval Zanella
0 siblings, 1 reply; 27+ messages in thread
From: Zack Weinberg @ 2016-08-19 16:06 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Fri, Aug 19, 2016 at 10:00 AM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 19/08/2016 10:00, Zack Weinberg wrote:
>>
>> OK, what should I be using instead?
>
> [... some options which I will look into ...]
>
> That's why I think simpler solution would be just to remove this inline
> optimization.
It's really, really not simpler, though, because of the fortify issue...
>> The *other* reason why I want to keep the inline optimization is that it
>> means I don't need to add __explicit_bzero_chk (which, because of
>> ifuncs, means futzing with *every sysdeps definition of memset*!). So
>> either way there's going to be a new exported symbol.
>
> I do not think this is really required, you can still add a default
> debug/explicit_bzero_chk.c which will call explicit_bzero/memset.
You missed the point; the point was that if we don't add
__glibc_read_memory then we have to add __explicit_bzero_chk, so
there's one new implementation-namespace exported symbol either way.
Anyway, it would be nice if that worked without modifying every
definition of memset, but it doesn't. The problem is not that
__explicit_bzero_chk itself would need to be an ifunc (I agree that
that would be an unnecessary optimization); the problem is that
__explicit_bzero_chk would need to call __memset_chk from inside
libc.so. There are currently no calls to __memset_chk inside libc.so,
which means there is no PLT bypass glue for __memset_chk. And because
_some_ architectures define __memset_chk as an ifunc, it's in sysdeps,
and _every_ memset.S has to be audited and possibly touched. This is
such a PITA that in
https://sourceware.org/ml/libc-alpha/2015-11/msg00467.html I appear to
have only done it for x86.
With the inline optimization, the fortified explicit_bzero can live
entirely within bits/string3.h and no modifications to __memset_chk
are required.
zw
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-19 16:06 ` Zack Weinberg
@ 2016-08-19 16:32 ` Adhemerval Zanella
2016-08-19 19:54 ` Zack Weinberg
0 siblings, 1 reply; 27+ messages in thread
From: Adhemerval Zanella @ 2016-08-19 16:32 UTC (permalink / raw)
To: Zack Weinberg; +Cc: GNU C Library
On 19/08/2016 13:06, Zack Weinberg wrote:
> On Fri, Aug 19, 2016 at 10:00 AM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 19/08/2016 10:00, Zack Weinberg wrote:
>>>
>>> OK, what should I be using instead?
>>
>> [... some options which I will look into ...]
>>
>> That's why I think simpler solution would be just to remove this inline
>> optimization.
>
> It's really, really not simpler, though, because of the fortify issue...
>
>>> The *other* reason why I want to keep the inline optimization is that it
>>> means I don't need to add __explicit_bzero_chk (which, because of
>>> ifuncs, means futzing with *every sysdeps definition of memset*!). So
>>> either way there's going to be a new exported symbol.
>>
>> I do not think this is really required, you can still add a default
>> debug/explicit_bzero_chk.c which will call explicit_bzero/memset.
>
> You missed the point; the point was that if we don't add
> __glibc_read_memory then we have to add __explicit_bzero_chk, so
> there's one new implementation-namespace exported symbol either way.
>
> Anyway, it would be nice if that worked without modifying every
> definition of memset, but it doesn't. The problem is not that
> __explicit_bzero_chk itself would need to be an ifunc (I agree that
> that would be an unnecessary optimization); the problem is that
> __explicit_bzero_chk would need to call __memset_chk from inside
> libc.so. There are currently no calls to __memset_chk inside libc.so,
> which means there is no PLT bypass glue for __memset_chk. And because
> _some_ architectures define __memset_chk as an ifunc, it's in sysdeps,
> and _every_ memset.S has to be audited and possibly touched. This is
> such a PITA that in
> https://sourceware.org/ml/libc-alpha/2015-11/msg00467.html I appear to
> have only done it for x86.
>
> With the inline optimization, the fortified explicit_bzero can live
> entirely within bits/string3.h and no modifications to __memset_chk
> are required.
Well, current memset_chk default implementation just does:
if (__glibc_unlikely (dstlen < len))
__chk_fail ();
So it would be feasible imho to just this add and call memset instead
of memset_chk.
The problem, as pointed out by Florian, is not the symbol being exported
itself, but the inline definition where all the 'inline' definition might
either not being supported by the target compiler or have non-expected
side effects (as for C++ with -O0).
The fortified version is mostly fine to use 'inline' definition because
it is already guarded by _FORTIFY_SOURCE, but I think the default
explicit_bzero guarded by __USE_MISC is not suffice.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-19 16:32 ` Adhemerval Zanella
@ 2016-08-19 19:54 ` Zack Weinberg
2016-08-19 21:16 ` Adhemerval Zanella
0 siblings, 1 reply; 27+ messages in thread
From: Zack Weinberg @ 2016-08-19 19:54 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Fri, Aug 19, 2016 at 12:31 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> Well, current memset_chk default implementation just does:
>
> if (__glibc_unlikely (dstlen < len))
> __chk_fail ();
>
> So it would be feasible imho to just this add and call memset instead
> of memset_chk.
For clarity, you are imagining this as the in-libc, out-of-line
implementation of __explicit_bzero_chk? Because as long as there's
*some* inline definition of explicit_bzero, we have to have the
exported __glibc_read_memory and then I don't see why we shouldn't
have a string2.h non-fortified inline as well.
> The problem, as pointed out by Florian, is not the symbol being exported
> itself, but the inline definition where all the 'inline' definition might
> either not being supported by the target compiler or have non-expected
> side effects (as for C++ with -O0).
Did you miss that there *is* an out-of-line non-fortified
explicit_bzero? Which means that the bug you pointed out should be a
non-issue.
zw
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-19 19:54 ` Zack Weinberg
@ 2016-08-19 21:16 ` Adhemerval Zanella
0 siblings, 0 replies; 27+ messages in thread
From: Adhemerval Zanella @ 2016-08-19 21:16 UTC (permalink / raw)
To: Zack Weinberg; +Cc: GNU C Library
On 19/08/2016 16:54, Zack Weinberg wrote:
> On Fri, Aug 19, 2016 at 12:31 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> Well, current memset_chk default implementation just does:
>>
>> if (__glibc_unlikely (dstlen < len))
>> __chk_fail ();
>>
>> So it would be feasible imho to just this add and call memset instead
>> of memset_chk.
>
> For clarity, you are imagining this as the in-libc, out-of-line
> implementation of __explicit_bzero_chk? Because as long as there's
> *some* inline definition of explicit_bzero, we have to have the
> exported __glibc_read_memory and then I don't see why we shouldn't
> have a string2.h non-fortified inline as well.
What I would like is, based on recent string inline headers cleanup,
is to avoid adding more implementation that in the future the compiler
can and would handle this transparently. Ideally it would be better if
we just aims to clean all the optimization done in string2/string3
headers (as we did for recent strspn, strbrk, etc. optimizations).
For this special case, I would advise go for the simple case: add a
explicit_bzero and __explicit_bzero_chk in-libc as the other implementations.
However I do not have a strong opinion here.
>
>> The problem, as pointed out by Florian, is not the symbol being exported
>> itself, but the inline definition where all the 'inline' definition might
>> either not being supported by the target compiler or have non-expected
>> side effects (as for C++ with -O0).
>
> Did you miss that there *is* an out-of-line non-fortified
> explicit_bzero? Which means that the bug you pointed out should be a
> non-issue.
The problem is not the existence of a out-of-line explicit_bzero, but
if __extern_inline would use or not the gnu_inline attribute. With current
guards for inclusion of string2.h (OPTIMIZE and !__cplusplus) the only
case of a possible issue I can think of is if someone building with
GCC lower than 4.2 with an optimized build and -fno-inline (this will
define __extern_inline to just extern __inline and possible create
multiple definitions). My inclination is this should not be a blocker
though.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/4] New string function explicit_bzero (from OpenBSD).
2016-08-18 20:53 ` Torvald Riegel
2016-08-19 12:44 ` Adhemerval Zanella
@ 2016-08-19 12:50 ` Florian Weimer
1 sibling, 0 replies; 27+ messages in thread
From: Florian Weimer @ 2016-08-19 12:50 UTC (permalink / raw)
To: Torvald Riegel; +Cc: Zack Weinberg, libc-alpha
On 08/18/2016 10:53 PM, Torvald Riegel wrote:
> On Thu, 2016-08-18 at 20:31 +0200, Florian Weimer wrote:
>> I don't think we want explicit_bzero to be inlined, it's useful to have
>> this name in the executable. Furthermore, we might want to add
>> additional state clearing later, so an implementation in libc.so.6 seems
>> desirable anyway.
>>
>> For an implementation in libc, there is currently no different between
>> the __glibc_read_memory kludge and a full memory barrier, so I suggest
>> to go with the latter. (The explicit_bzero call will serve as a rather
>> broad barrier anyway, but we can annotate it with __THROW.)
>
> I suppose we just want a compiler barrier here though, and don't need a
> memory barrier in the sense of something that constrains HW reordering.
Yes, I meant an asm statement with a "memory" clobber.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 4/4] Use explicit_bzero where appropriate
2016-08-17 17:19 [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
` (2 preceding siblings ...)
2016-08-17 17:19 ` [PATCH 2/4] New string function explicit_bzero (from OpenBSD) Zack Weinberg
@ 2016-08-17 17:19 ` Zack Weinberg
2016-08-17 18:04 ` [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
` (2 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Zack Weinberg @ 2016-08-17 17:19 UTC (permalink / raw)
To: libc-alpha; +Cc: fweimer
I *believe* these are the only places where memset was being used
to clear buffers containing sensitive data. The compiler probably
couldn't optimize *all* of them out but it seems best to change them all.
The legacy DES implementation wasn't bothering to clear its buffers,
so I added that, mostly for consistency's sake.
* crypt/crypt-entry.c (__crypt_r): Clear key-dependent intermediate
data before returning, using explicit_bzero.
* crypt/md5-crypt.c (__md5_crypt_r): Likewise.
* crypt/sha256-crypt.c (__sha256_crypt_r): Likewise.
* crypt/sha512-crypt.c (__sha512_crypt_r): Likewise.
---
crypt/crypt-entry.c | 11 +++++++++++
crypt/md5-crypt.c | 8 ++++----
crypt/sha256-crypt.c | 14 +++++++-------
crypt/sha512-crypt.c | 14 +++++++-------
4 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/crypt/crypt-entry.c b/crypt/crypt-entry.c
index a7dfcca..09a5664 100644
--- a/crypt/crypt-entry.c
+++ b/crypt/crypt-entry.c
@@ -141,6 +141,17 @@ __crypt_r (const char *key, const char *salt,
* And convert back to 6 bit ASCII
*/
_ufc_output_conversion_r (res[0], res[1], salt, data);
+
+#ifdef _LIBC
+ /*
+ * Erase key-dependent intermediate data. Data dependent only on
+ * the salt is not considered sensitive.
+ */
+ explicit_bzero (ktab, sizeof (ktab));
+ explicit_bzero (data->keysched, sizeof (data->keysched));
+ explicit_bzero (res, sizeof (res));
+#endif
+
return data->crypt_3_buf;
}
weak_alias (__crypt_r, crypt_r)
diff --git a/crypt/md5-crypt.c b/crypt/md5-crypt.c
index 2243bc7..6125c7f 100644
--- a/crypt/md5-crypt.c
+++ b/crypt/md5-crypt.c
@@ -288,13 +288,13 @@ __md5_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
#ifndef USE_NSS
__md5_init_ctx (&ctx);
__md5_finish_ctx (&ctx, alt_result);
- memset (&ctx, '\0', sizeof (ctx));
- memset (&alt_ctx, '\0', sizeof (alt_ctx));
+ explicit_bzero (&ctx, sizeof (ctx));
+ explicit_bzero (&alt_ctx, sizeof (alt_ctx));
#endif
if (copied_key != NULL)
- memset (copied_key, '\0', key_len);
+ explicit_bzero (copied_key, key_len);
if (copied_salt != NULL)
- memset (copied_salt, '\0', salt_len);
+ explicit_bzero (copied_salt, salt_len);
free (free_key);
return buffer;
diff --git a/crypt/sha256-crypt.c b/crypt/sha256-crypt.c
index ca703de..6717e22 100644
--- a/crypt/sha256-crypt.c
+++ b/crypt/sha256-crypt.c
@@ -371,16 +371,16 @@ __sha256_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
#ifndef USE_NSS
__sha256_init_ctx (&ctx);
__sha256_finish_ctx (&ctx, alt_result);
- memset (&ctx, '\0', sizeof (ctx));
- memset (&alt_ctx, '\0', sizeof (alt_ctx));
+ explicit_bzero (&ctx, sizeof (ctx));
+ explicit_bzero (&alt_ctx, sizeof (alt_ctx));
#endif
- memset (temp_result, '\0', sizeof (temp_result));
- memset (p_bytes, '\0', key_len);
- memset (s_bytes, '\0', salt_len);
+ explicit_bzero (temp_result, sizeof (temp_result));
+ explicit_bzero (p_bytes, key_len);
+ explicit_bzero (s_bytes, salt_len);
if (copied_key != NULL)
- memset (copied_key, '\0', key_len);
+ explicit_bzero (copied_key, key_len);
if (copied_salt != NULL)
- memset (copied_salt, '\0', salt_len);
+ explicit_bzero (copied_salt, salt_len);
free (free_key);
free (free_pbytes);
diff --git a/crypt/sha512-crypt.c b/crypt/sha512-crypt.c
index c42e5b7..1965191 100644
--- a/crypt/sha512-crypt.c
+++ b/crypt/sha512-crypt.c
@@ -393,16 +393,16 @@ __sha512_crypt_r (const char *key, const char *salt, char *buffer, int buflen)
#ifndef USE_NSS
__sha512_init_ctx (&ctx);
__sha512_finish_ctx (&ctx, alt_result);
- memset (&ctx, '\0', sizeof (ctx));
- memset (&alt_ctx, '\0', sizeof (alt_ctx));
+ explicit_bzero (&ctx, sizeof (ctx));
+ explicit_bzero (&alt_ctx, sizeof (alt_ctx));
#endif
- memset (temp_result, '\0', sizeof (temp_result));
- memset (p_bytes, '\0', key_len);
- memset (s_bytes, '\0', salt_len);
+ explicit_bzero (temp_result, sizeof (temp_result));
+ explicit_bzero (p_bytes, key_len);
+ explicit_bzero (s_bytes, salt_len);
if (copied_key != NULL)
- memset (copied_key, '\0', key_len);
+ explicit_bzero (copied_key, key_len);
if (copied_salt != NULL)
- memset (copied_salt, '\0', salt_len);
+ explicit_bzero (copied_salt, salt_len);
free (free_key);
free (free_pbytes);
--
2.9.3
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-08-17 17:19 [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
` (3 preceding siblings ...)
2016-08-17 17:19 ` [PATCH 4/4] Use explicit_bzero where appropriate Zack Weinberg
@ 2016-08-17 18:04 ` Zack Weinberg
2016-08-18 17:49 ` Mike Frysinger
2016-08-17 22:20 ` Manuel López-Ibáñez
2016-09-06 17:11 ` Zack Weinberg
6 siblings, 1 reply; 27+ messages in thread
From: Zack Weinberg @ 2016-08-17 18:04 UTC (permalink / raw)
To: GNU C Library
On Wed, Aug 17, 2016 at 1:19 PM, Zack Weinberg <zackw@panix.com> wrote:
> Now that we're in full swing on 2.25 (and I'm not trying to hit a
> conference deadline), here is an updated version of the explicit_bzero
> patchset.
I *may* have now published this as the zack/explicit-bzero branch in
glibc.git. I got a weird "Invalid revision range" error.
zw
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-08-17 17:19 [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
` (4 preceding siblings ...)
2016-08-17 18:04 ` [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
@ 2016-08-17 22:20 ` Manuel López-Ibáñez
2016-08-18 16:30 ` Zack Weinberg
2016-09-06 17:11 ` Zack Weinberg
6 siblings, 1 reply; 27+ messages in thread
From: Manuel López-Ibáñez @ 2016-08-17 22:20 UTC (permalink / raw)
To: Zack Weinberg, libc-alpha
On 17/08/16 18:19, Zack Weinberg wrote:
> anything I would like to hear about it. If anyone has any ideas for
> how to write a test that will start failing if a compiler ever learns
> to "see through" __glibc_read_memory, I would like to hear about that,
> too. (I can imagine a way to do it with gcc's scan-assembler tests,
> but we don't have those here.)
Perhaps? https://plus.google.com/+MatthewDempsky/posts/KQHFBouxurX
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-08-17 22:20 ` Manuel López-Ibáñez
@ 2016-08-18 16:30 ` Zack Weinberg
2016-08-18 17:51 ` Mike Frysinger
2016-08-18 18:18 ` Florian Weimer
0 siblings, 2 replies; 27+ messages in thread
From: Zack Weinberg @ 2016-08-18 16:30 UTC (permalink / raw)
To: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 878 bytes --]
On 08/17/2016 06:20 PM, Manuel López-Ibáñez wrote:
> On 17/08/16 18:19, Zack Weinberg wrote:
>> anything I would like to hear about it. If anyone has any ideas for
>> how to write a test that will start failing if a compiler ever learns
>> to "see through" __glibc_read_memory, I would like to hear about that,
>> too. (I can imagine a way to do it with gcc's scan-assembler tests,
>> but we don't have those here.)
>
> Perhaps? https://plus.google.com/+MatthewDempsky/posts/KQHFBouxurX
Ingenious! Done - patch 5/4 attached to this message, and also included
in zack/explicit-bzero. I would fold this into patch 2 for landing, but
it may be easier to review this way.
(What is the preferred way to deal with user-namespace project branches
that get rebased? The server won't let me push -f; I can delete the
branch and recreate it but that seems ... not great.)
zw
[-- Attachment #2: 0001-Add-a-test-for-explicit_bzero-not-getting-optimized-.patch --]
[-- Type: text/x-patch, Size: 13942 bytes --]
From a4eb2c453d93e566f00a9f67f6775a51a2eef9fc Mon Sep 17 00:00:00 2001
From: Zack Weinberg <zackw@panix.com>
Date: Thu, 18 Aug 2016 12:22:55 -0400
Subject: [PATCH] Add a test for explicit_bzero not getting optimized out.
Conceptually based on a test written by Matthew Dempsky for the OpenBSD
regression suite.
* string/tst-xbzero-opt.c: New test.
* string/Makefile: Run it.
---
string/Makefile | 2 +-
string/tst-xbzero-opt.c | 383 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 384 insertions(+), 1 deletion(-)
create mode 100644 string/tst-xbzero-opt.c
diff --git a/string/Makefile b/string/Makefile
index 180ec86..be7154b 100644
--- a/string/Makefile
+++ b/string/Makefile
@@ -60,7 +60,7 @@ tests := tester inl-tester noinl-tester testcopy test-ffs \
tst-strtok tst-strxfrm bug-strcoll1 tst-strfry \
bug-strtok1 $(addprefix test-,$(strop-tests)) \
bug-envz1 tst-strxfrm2 tst-endian tst-svc2 \
- tst-strtok_r bug-strcoll2
+ tst-strtok_r bug-strcoll2 tst-xbzero-opt
xtests = tst-strcoll-overflow
diff --git a/string/tst-xbzero-opt.c b/string/tst-xbzero-opt.c
new file mode 100644
index 0000000..81eb5c1
--- /dev/null
+++ b/string/tst-xbzero-opt.c
@@ -0,0 +1,383 @@
+/* Test that explicit_bzero block clears are not optimized out.
+ Copyright (C) 2016 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* This test is conceptually based on a test designed by Matthew
+ Dempsky for the OpenBSD regression suite:
+ <openbsd>/src/regress/lib/libc/explicit_bzero/explicit_bzero.c.
+ The basic idea is, we have a function that contains a
+ block-clearing operation (not necessarily explicit_bzero), after
+ which the block is dead, in the compiler-jargon sense. Execute
+ that function from a signal handler running on an alternative
+ signal stack. Then we have another pointer to the memory region
+ affected by the block clear -- namely, the sigaltstack buffer --
+ and can find out whether it actually happened.
+
+ The OpenBSD test cautions that some operating systems (e.g. Solaris
+ and OSX) wipe the signal stack upon returning to the normal stack,
+ so the test has to happen while still executing on the signal
+ stack. This, of course, means that the buffer might be clobbered
+ by normal stack operations after the function with the block clear
+ returns (it has to return, so that the block is truly dead). The
+ most straightforward way to deal with this is to have a large block
+ containing several copies of a byte pattern that is unlikely to
+ occur by chance, and check whether _any_ of them survives. */
+
+#define _GNU_SOURCE 1
+
+#include <stdbool.h>
+#include <errno.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+/* The "byte pattern that is unlikely to occur by chance": the first
+ 16 prime numbers (OEIS A000040). */
+static const unsigned char test_pattern[16] =
+{
+ 2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53
+};
+
+#define PATTERN_SIZE (sizeof test_pattern)
+#define PATTERN_REPS 32
+#define TEST_BUFFER_SIZE (PATTERN_SIZE * PATTERN_REPS)
+
+static void
+fill_with_test_pattern (unsigned char *buf)
+{
+ for (unsigned int i = 0; i < PATTERN_REPS; i++)
+ memcpy (buf + i*PATTERN_SIZE, test_pattern, PATTERN_SIZE);
+}
+
+static unsigned int
+count_test_patterns (unsigned char *buf, size_t bufsiz)
+{
+ unsigned char *first = memmem (buf, bufsiz, test_pattern, PATTERN_SIZE);
+ if (!first)
+ return 0;
+ unsigned int cnt = 0;
+ for (unsigned int i = 0; i < PATTERN_REPS; i++)
+ {
+ unsigned char *p = first + i*PATTERN_SIZE;
+ if (p + PATTERN_SIZE - buf > bufsiz)
+ break;
+ if (memcmp (p, test_pattern, PATTERN_SIZE) == 0)
+ cnt++;
+ }
+ return cnt;
+}
+
+/* Global test state. */
+static int failed_subtests;
+static bool this_subtest_failed;
+
+/* The signal stack is allocated with memalign. */
+static unsigned char *signal_stack_buffer;
+#define SIGNAL_STACK_SIZE (SIGSTKSZ + TEST_BUFFER_SIZE)
+
+enum test_expectation { EXPECT_NONE, EXPECT_SOME, EXPECT_ALL };
+
+static void
+check_test_buffer (enum test_expectation expected,
+ const char *label, const char *stage)
+{
+ unsigned int cnt = count_test_patterns (signal_stack_buffer,
+ SIGNAL_STACK_SIZE);
+ switch (expected)
+ {
+ case EXPECT_NONE:
+ if (cnt == 0)
+ {
+ printf ("PASS: %s/%s: expected 0 got %d\n", label, stage, cnt);
+ fflush (stdout);
+ }
+ else
+ {
+ printf ("FAIL: %s/%s: expected 0 got %d\n", label, stage, cnt);
+ fflush (stdout);
+ this_subtest_failed = true;
+ failed_subtests++;
+ }
+ break;
+
+ case EXPECT_SOME:
+ if (cnt > 0)
+ {
+ printf ("PASS: %s/%s: expected some got %d\n", label, stage, cnt);
+ fflush (stdout);
+ }
+ else
+ {
+ printf ("FAIL: %s/%s: expected some got 0\n", label, stage);
+ fflush (stdout);
+ this_subtest_failed = true;
+ failed_subtests++;
+ }
+ break;
+
+ case EXPECT_ALL:
+ if (cnt == PATTERN_REPS)
+ {
+ printf ("PASS: %s/%s: expected %d got %d\n", label, stage,
+ PATTERN_REPS, cnt);
+ fflush (stdout);
+ }
+ else
+ {
+ printf ("FAIL: %s/%s: expected %d got %d\n", label, stage,
+ PATTERN_REPS, cnt);
+ fflush (stdout);
+ this_subtest_failed = true;
+ failed_subtests++;
+ }
+ break;
+
+ default:
+ printf ("ERROR: %s/%s: invalid value for 'expected' = %d\n",
+ label, stage, (int)expected);
+ fflush (stdout);
+ this_subtest_failed = true;
+ failed_subtests++;
+ }
+}
+
+/* Always check the test buffer immediately after filling it; this
+ makes externally visible side effects depend on the buffer existing
+ and having been filled in. */
+static void
+prepare_test_buffer (unsigned char *buf, const char *label)
+{
+ fill_with_test_pattern (buf);
+ check_test_buffer (EXPECT_ALL, label, "prepare");
+
+ unsigned char *loc = memmem (signal_stack_buffer, SIGNAL_STACK_SIZE,
+ test_pattern, PATTERN_SIZE);
+ if (loc == buf)
+ {
+ printf ("PASS: %s/prepare: expected buffer location %p got %p\n",
+ label, buf, loc);
+ fflush (stdout);
+ }
+ else
+ {
+ printf ("FAIL: %s/prepare: expected buffer location %p got %p\n",
+ label, buf, loc);
+ fflush (stdout);
+ this_subtest_failed = true;
+ failed_subtests++;
+ }
+}
+
+/* There are three subtests, two of which are sanity checks.
+
+ In the "no_clear" case, we don't do anything to the test buffer
+ between preparing it and letting it go out of scope, and we expect
+ to find it. This confirms that the test buffer does get filled in
+ and we can find it from the stack buffer. In the "ordinary_clear"
+ case, we clear it using memset, and we expect to find it. This
+ confirms that the compiler can optimize out block clears in this
+ context; if it can't, the real test might be succeeding for the
+ wrong reason. Finally, the "explicit_clear" case uses
+ explicit_bzero and expects _not_ to find the test buffer, which is
+ the real test. */
+
+static void
+setup_no_clear (void)
+{
+ unsigned char buf[TEST_BUFFER_SIZE];
+ prepare_test_buffer (buf, "no clear");
+}
+
+static void
+setup_ordinary_clear (void)
+{
+ unsigned char buf[TEST_BUFFER_SIZE];
+ prepare_test_buffer (buf, "ordinary clear");
+ if (this_subtest_failed)
+ return;
+ memset (buf, 0, TEST_BUFFER_SIZE);
+}
+
+static void
+setup_explicit_clear (void)
+{
+ unsigned char buf[TEST_BUFFER_SIZE];
+ prepare_test_buffer (buf, "explicit clear");
+ if (this_subtest_failed)
+ return;
+ explicit_bzero (buf, TEST_BUFFER_SIZE);
+}
+
+struct subtest
+{
+ void (*setup_subtest) (void);
+ const char *label;
+ enum test_expectation expected;
+};
+
+static const struct subtest subtests[] =
+{
+ { setup_no_clear, "no clear", EXPECT_SOME },
+ { setup_ordinary_clear, "ordinary clear", EXPECT_SOME },
+ { setup_explicit_clear, "explicit clear", EXPECT_NONE },
+ { 0, 0, -1 }
+};
+static const struct subtest *this_subtest;
+static bool this_subtest_complete;
+
+/* This function is called as a signal handler. The signal is
+ triggered by a call to raise, therefore it is safe to do
+ non-async-signal-safe things within this function.
+
+ The this_subtest_complete flag addresses a race. The outer loop
+ keeps SIGUSR1 blocked all the time, unblocking it only immediately
+ after setting up the appropriate conditions for a test and then
+ raising SIGUSR1 itself. SIGUSR1 is not a real-time signal, so if
+ another process sends this process SIGUSR1 _before_ it's unblocked
+ by the outer loop, this function will only be called once.
+ However, if another process sends this process SIGUSR1 _while this
+ handler is already running_, that signal will be pending upon
+ return from this function, and will fire before the outer loop has
+ a chance to re-block SIGUSR1. This is unavoidable; the workaround
+ is to arrange for this function not to do anything if it's called
+ several times in a row. */
+static void
+do_subtest (int signo __attribute__ ((unused)))
+{
+ if (!this_subtest_complete)
+ {
+ this_subtest->setup_subtest ();
+ if (!this_subtest_failed)
+ check_test_buffer (this_subtest->expected,
+ this_subtest->label,
+ "test");
+ this_subtest_complete = true;
+ }
+}
+
+static int
+do_test (void)
+{
+ /* test-skeleton.c unconditionally sets stdout to be unbuffered.
+ vfprintf allocates a great deal of memory on the stack if called
+ with an unbuffered FILE*, overflowing the alt-stack. This is
+ also why there is a call to fflush after every call to printf in
+ this file. */
+ if (setvbuf (stdout, 0, _IOFBF, 0))
+ {
+ printf ("ERROR: restoring stdout buffering: %s\n", strerror (errno));
+ fflush (stdout);
+ return 2;
+ }
+
+ size_t page_alignment = sysconf (_SC_PAGESIZE);
+ if (page_alignment < sizeof (void *))
+ page_alignment = sizeof (void *);
+
+ void *p;
+ int err = posix_memalign (&p, page_alignment, SIGNAL_STACK_SIZE);
+ if (err || !p)
+ {
+ printf ("ERROR: allocating alt stack: %s\n", strerror (err));
+ fflush (stdout);
+ return 2;
+ }
+ signal_stack_buffer = p;
+
+ /* This program will malfunction if it receives SIGUSR1 signals at any
+ time other than when it's just sent one to itself. Therefore, keep
+ it blocked most of the time. */
+ sigset_t sigusr1_mask;
+ sigemptyset (&sigusr1_mask);
+ sigaddset (&sigusr1_mask, SIGUSR1);
+ if (sigprocmask (SIG_BLOCK, &sigusr1_mask, 0))
+ {
+ printf ("ERROR: block(SIGUSR1): %s\n", strerror (errno));
+ fflush (stdout);
+ return 2;
+ }
+
+ stack_t ss;
+ ss.ss_sp = signal_stack_buffer;
+ ss.ss_flags = 0;
+ ss.ss_size = SIGNAL_STACK_SIZE;
+ if (sigaltstack (&ss, 0))
+ {
+ printf ("ERROR: sigaltstack: %s\n", strerror (errno));
+ fflush (stdout);
+ return 2;
+ }
+
+ struct sigaction sa;
+ sigemptyset (&sa.sa_mask);
+ sigaddset (&sa.sa_mask, SIGUSR1);
+ sa.sa_handler = do_subtest;
+ sa.sa_flags = SA_RESTART | SA_ONSTACK;
+ if (sigaction (SIGUSR1, &sa, 0))
+ {
+ printf ("ERROR: sigaction(SIGUSR1): %s\n", strerror (errno));
+ fflush (stdout);
+ return 2;
+ }
+
+ /* We use kill instead of raise, because raise screws with the
+ signal mask, which we don't want. */
+ pid_t self = getpid ();
+
+ this_subtest = subtests;
+ while (this_subtest->label)
+ {
+ this_subtest_complete = false;
+ this_subtest_failed = false;
+
+ /* Completely clear the signal stack between tests, so that junk
+ from previous tests cannot interfere with the current one. */
+ memset (signal_stack_buffer, 0, SIGNAL_STACK_SIZE);
+
+ /* Raise SIGUSR1, then unblock it, then immediately block it
+ again. This will trigger do_subtest to run _once_ on the
+ alternative stack, at the point of calling sigprocmask to
+ unblock the signal. */
+ if (kill (self, SIGUSR1))
+ {
+ printf ("ERROR: raise(SIGUSR1): %s\n", strerror (errno));
+ fflush (stdout);
+ return 2;
+ }
+ if (sigprocmask (SIG_UNBLOCK, &sigusr1_mask, 0))
+ {
+ printf ("ERROR: unblock(SIGUSR1): %s\n", strerror (errno));
+ fflush (stdout);
+ return 2;
+ }
+ if (sigprocmask (SIG_BLOCK, &sigusr1_mask, 0))
+ {
+ printf ("ERROR: unblock(SIGUSR1): %s\n", strerror(errno));
+ fflush (stdout);
+ return 2;
+ }
+
+ this_subtest++;
+ }
+
+ return failed_subtests ? 1 : 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
--
2.9.3
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-08-18 16:30 ` Zack Weinberg
@ 2016-08-18 17:51 ` Mike Frysinger
2016-08-18 18:18 ` Florian Weimer
1 sibling, 0 replies; 27+ messages in thread
From: Mike Frysinger @ 2016-08-18 17:51 UTC (permalink / raw)
To: Zack Weinberg; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 385 bytes --]
On 18 Aug 2016 12:29, Zack Weinberg wrote:
> (What is the preferred way to deal with user-namespace project branches
> that get rebased? The server won't let me push -f; I can delete the
> branch and recreate it but that seems ... not great.)
seems like we should just allow force pushes on user branches. probably
need to file a bug for that though, or e-mail the overseers.
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-08-18 16:30 ` Zack Weinberg
2016-08-18 17:51 ` Mike Frysinger
@ 2016-08-18 18:18 ` Florian Weimer
2016-08-19 12:48 ` Zack Weinberg
1 sibling, 1 reply; 27+ messages in thread
From: Florian Weimer @ 2016-08-18 18:18 UTC (permalink / raw)
To: Zack Weinberg, libc-alpha
On 08/18/2016 06:29 PM, Zack Weinberg wrote:
> On 08/17/2016 06:20 PM, Manuel López-Ibáñez wrote:
>> On 17/08/16 18:19, Zack Weinberg wrote:
>>> anything I would like to hear about it. If anyone has any ideas for
>>> how to write a test that will start failing if a compiler ever learns
>>> to "see through" __glibc_read_memory, I would like to hear about that,
>>> too. (I can imagine a way to do it with gcc's scan-assembler tests,
>>> but we don't have those here.)
>>
>> Perhaps? https://plus.google.com/+MatthewDempsky/posts/KQHFBouxurX
>
> Ingenious! Done - patch 5/4 attached to this message, and also included
> in zack/explicit-bzero. I would fold this into patch 2 for landing, but
> it may be easier to review this way.
GCC scans the generated assembly for such things. It seems more
reliable, particularly if we retain the explicit_bzero symbol.
> (What is the preferred way to deal with user-namespace project branches
> that get rebased? The server won't let me push -f; I can delete the
> branch and recreate it but that seems ... not great.)
Yes, we should stop rebasing only on master and release/* branches.
I'll send a separate mail about that.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-08-18 18:18 ` Florian Weimer
@ 2016-08-19 12:48 ` Zack Weinberg
0 siblings, 0 replies; 27+ messages in thread
From: Zack Weinberg @ 2016-08-19 12:48 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 08/18/2016 02:18 PM, Florian Weimer wrote:
> On 08/18/2016 06:29 PM, Zack Weinberg wrote:
>> On 08/17/2016 06:20 PM, Manuel López-Ibáñez wrote:
>>> On 17/08/16 18:19, Zack Weinberg wrote:
>>>> anything I would like to hear about it. If anyone has any ideas for
>>>> how to write a test that will start failing if a compiler ever learns
>>>> to "see through" __glibc_read_memory, I would like to hear about that,
>>>> too. (I can imagine a way to do it with gcc's scan-assembler tests,
>>>> but we don't have those here.)
>>>
>>> Perhaps? https://plus.google.com/+MatthewDempsky/posts/KQHFBouxurX
>>
>> Ingenious! Done - patch 5/4 attached to this message, and also included
>> in zack/explicit-bzero. I would fold this into patch 2 for landing, but
>> it may be easier to review this way.
>
> GCC scans the generated assembly for such things. It seems more
> reliable, particularly if we retain the explicit_bzero symbol.
Unlike GCC we don't have any infrastructure for doing that, though, and
if we *don't* retain the explicit_bzero symbol (see other message) we
would have to detect all possible inline memset constructs, which
doesn't sound fun.
zw
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-08-17 17:19 [PATCH 0/4] explicit_bzero, this time for sure Zack Weinberg
` (5 preceding siblings ...)
2016-08-17 22:20 ` Manuel López-Ibáñez
@ 2016-09-06 17:11 ` Zack Weinberg
2016-09-07 9:34 ` Florian Weimer
6 siblings, 1 reply; 27+ messages in thread
From: Zack Weinberg @ 2016-09-06 17:11 UTC (permalink / raw)
To: GNU C Library
On Wed, Aug 17, 2016 at 1:19 PM, Zack Weinberg <zackw@panix.com> wrote:
> Now that we're in full swing on 2.25 (and I'm not trying to hit a
> conference deadline), here is an updated version of the explicit_bzero
> patchset.
Ping.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-09-06 17:11 ` Zack Weinberg
@ 2016-09-07 9:34 ` Florian Weimer
2016-09-07 13:05 ` Zack Weinberg
0 siblings, 1 reply; 27+ messages in thread
From: Florian Weimer @ 2016-09-07 9:34 UTC (permalink / raw)
To: Zack Weinberg, GNU C Library
On 09/06/2016 07:11 PM, Zack Weinberg wrote:
> On Wed, Aug 17, 2016 at 1:19 PM, Zack Weinberg <zackw@panix.com> wrote:
>> Now that we're in full swing on 2.25 (and I'm not trying to hit a
>> conference deadline), here is an updated version of the explicit_bzero
>> patchset.
>
> Ping.
I think the consensus evolved that we want an explict_bzero and
__explicit_bzero_chk in libc, not just the inline wrapper.
Thanks,
Florian
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/4] explicit_bzero, this time for sure
2016-09-07 9:34 ` Florian Weimer
@ 2016-09-07 13:05 ` Zack Weinberg
0 siblings, 0 replies; 27+ messages in thread
From: Zack Weinberg @ 2016-09-07 13:05 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
On Wed, Sep 7, 2016 at 5:34 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 09/06/2016 07:11 PM, Zack Weinberg wrote:
>> On Wed, Aug 17, 2016 at 1:19 PM, Zack Weinberg <zackw@panix.com> wrote:
>>> Now that we're in full swing on 2.25 (and I'm not trying to hit a
>>> conference deadline), here is an updated version of the explicit_bzero
>>> patchset.
>>
>> Ping.
>
> I think the consensus evolved that we want an explict_bzero and
> __explicit_bzero_chk in libc, not just the inline wrapper.
Yes, but that is essentially the way the patch already is! I can
reshuffle it to make that clearer but I would like to know whether
there's anything else that needs fixing first.
zw
^ permalink raw reply [flat|nested] 27+ messages in thread