Re: update explicit_bzero test to not assume SIGSTKSZ to be constant
On Fri, Mar 26, 2021 at 1:56 PM Alexander Bluhm wrote:
>
> On Mon, Mar 22, 2021 at 08:38:23PM -0500, Brent Cook wrote:
> > In the next version of Linux glibc, SIGSTKSZ is defined at runtime if
> > source is built with _GNU_SOURCE. On LibreSSL-portable, this is set to
> > bring in asprintf/vasprintf, which causes the explicit_bzero test to
> > fail to compile since the size of SIGSTKSZ is no longer known at compile
> > time. This adjusts the test to treat SIGSTKSZ as a runtime variable.
> >
> > See http://patches-tcwg.linaro.org/patch/48127/ and
> > https://github.com/libressl-portable/portable/issues/653 for the
> > LibreSSL build failure report on Fedora Rawhide.
> >
> > ok?
>
> OK bluhm@
>
> Could you put a comment there that SIGSTKSZ is not constant in GNU
> libc. Then someone reading the test knows why we malloc.
Thanks for all of the feedback! I just committed with added comments
and clarifications.
>
> > Index: explicit_bzero.c
> > ===
> > RCS file: /cvs/src/regress/lib/libc/explicit_bzero/explicit_bzero.c,v
> > retrieving revision 1.6
> > diff -u -p -u -p -r1.6 explicit_bzero.c
> > --- explicit_bzero.c 11 Jul 2014 01:10:35 - 1.6
> > +++ explicit_bzero.c 23 Mar 2021 01:32:21 -
> > @@ -18,6 +18,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -36,16 +37,20 @@ enum {
> > SECRETBYTES = SECRETCOUNT * sizeof(secret)
> > };
> >
> > -static char altstack[SIGSTKSZ + SECRETBYTES];
> > +static char *altstack;
> > +#define ALTSTACK_SIZE (SIGSTKSZ + SECRETBYTES)
> >
> > static void
> > setup_stack(void)
> > {
> > + altstack = malloc(ALTSTACK_SIZE);
> > +
> > const stack_t sigstk = {
> > .ss_sp = altstack,
> > - .ss_size = sizeof(altstack),
> > + .ss_size = ALTSTACK_SIZE
> > };
> >
> > + ASSERT_NE(NULL, altstack);
> > ASSERT_EQ(0, sigaltstack(&sigstk, NULL));
> > }
> >
> > @@ -129,7 +134,7 @@ test_without_bzero()
> > char buf[SECRETBYTES];
> > assert_on_stack();
> > populate_secret(buf, sizeof(buf));
> > - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> > + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> > ASSERT_NE(NULL, res);
> > return (res);
> > }
> > @@ -140,7 +145,7 @@ test_with_bzero()
> > char buf[SECRETBYTES];
> > assert_on_stack();
> > populate_secret(buf, sizeof(buf));
> > - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> > + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> > ASSERT_NE(NULL, res);
> > explicit_bzero(buf, sizeof(buf));
> > return (res);
> > @@ -183,14 +188,14 @@ main()
> >* on the stack. This sanity checks that call_on_stack() and
> >* populate_secret() work as intended.
> >*/
> > - memset(altstack, 0, sizeof(altstack));
> > + memset(altstack, 0, ALTSTACK_SIZE);
> > call_on_stack(do_test_without_bzero);
> >
> > /*
> >* Now test with a call to explicit_bzero() and check that we
> >* *don't* find any instances of the secret data.
> >*/
> > - memset(altstack, 0, sizeof(altstack));
> > + memset(altstack, 0, ALTSTACK_SIZE);
> > call_on_stack(do_test_with_bzero);
> >
> > return (0);
Re: update explicit_bzero test to not assume SIGSTKSZ to be constant
On Mon, Mar 22, 2021 at 08:38:23PM -0500, Brent Cook wrote:
> In the next version of Linux glibc, SIGSTKSZ is defined at runtime if
> source is built with _GNU_SOURCE. On LibreSSL-portable, this is set to
> bring in asprintf/vasprintf, which causes the explicit_bzero test to
> fail to compile since the size of SIGSTKSZ is no longer known at compile
> time. This adjusts the test to treat SIGSTKSZ as a runtime variable.
>
> See http://patches-tcwg.linaro.org/patch/48127/ and
> https://github.com/libressl-portable/portable/issues/653 for the
> LibreSSL build failure report on Fedora Rawhide.
>
> ok?
OK bluhm@
Could you put a comment there that SIGSTKSZ is not constant in GNU
libc. Then someone reading the test knows why we malloc.
> Index: explicit_bzero.c
> ===
> RCS file: /cvs/src/regress/lib/libc/explicit_bzero/explicit_bzero.c,v
> retrieving revision 1.6
> diff -u -p -u -p -r1.6 explicit_bzero.c
> --- explicit_bzero.c 11 Jul 2014 01:10:35 - 1.6
> +++ explicit_bzero.c 23 Mar 2021 01:32:21 -
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -36,16 +37,20 @@ enum {
> SECRETBYTES = SECRETCOUNT * sizeof(secret)
> };
>
> -static char altstack[SIGSTKSZ + SECRETBYTES];
> +static char *altstack;
> +#define ALTSTACK_SIZE (SIGSTKSZ + SECRETBYTES)
>
> static void
> setup_stack(void)
> {
> + altstack = malloc(ALTSTACK_SIZE);
> +
> const stack_t sigstk = {
> .ss_sp = altstack,
> - .ss_size = sizeof(altstack),
> + .ss_size = ALTSTACK_SIZE
> };
>
> + ASSERT_NE(NULL, altstack);
> ASSERT_EQ(0, sigaltstack(&sigstk, NULL));
> }
>
> @@ -129,7 +134,7 @@ test_without_bzero()
> char buf[SECRETBYTES];
> assert_on_stack();
> populate_secret(buf, sizeof(buf));
> - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> ASSERT_NE(NULL, res);
> return (res);
> }
> @@ -140,7 +145,7 @@ test_with_bzero()
> char buf[SECRETBYTES];
> assert_on_stack();
> populate_secret(buf, sizeof(buf));
> - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> ASSERT_NE(NULL, res);
> explicit_bzero(buf, sizeof(buf));
> return (res);
> @@ -183,14 +188,14 @@ main()
>* on the stack. This sanity checks that call_on_stack() and
>* populate_secret() work as intended.
>*/
> - memset(altstack, 0, sizeof(altstack));
> + memset(altstack, 0, ALTSTACK_SIZE);
> call_on_stack(do_test_without_bzero);
>
> /*
>* Now test with a call to explicit_bzero() and check that we
>* *don't* find any instances of the secret data.
>*/
> - memset(altstack, 0, sizeof(altstack));
> + memset(altstack, 0, ALTSTACK_SIZE);
> call_on_stack(do_test_with_bzero);
>
> return (0);
Re: update explicit_bzero test to not assume SIGSTKSZ to be constant
Fine with me.
The malloc'd memory is not identical to bss memory, but this still matches
the spirit of the test.
Brent Cook wrote:
> In the next version of Linux glibc, SIGSTKSZ is defined at runtime if
> source is built with _GNU_SOURCE. On LibreSSL-portable, this is set to
> bring in asprintf/vasprintf, which causes the explicit_bzero test to
> fail to compile since the size of SIGSTKSZ is no longer known at compile
> time. This adjusts the test to treat SIGSTKSZ as a runtime variable.
>
> See http://patches-tcwg.linaro.org/patch/48127/ and
> https://github.com/libressl-portable/portable/issues/653 for the
> LibreSSL build failure report on Fedora Rawhide.
>
> ok?
>
> Index: explicit_bzero.c
> ===
> RCS file: /cvs/src/regress/lib/libc/explicit_bzero/explicit_bzero.c,v
> retrieving revision 1.6
> diff -u -p -u -p -r1.6 explicit_bzero.c
> --- explicit_bzero.c 11 Jul 2014 01:10:35 - 1.6
> +++ explicit_bzero.c 23 Mar 2021 01:32:21 -
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -36,16 +37,20 @@ enum {
> SECRETBYTES = SECRETCOUNT * sizeof(secret)
> };
>
> -static char altstack[SIGSTKSZ + SECRETBYTES];
> +static char *altstack;
> +#define ALTSTACK_SIZE (SIGSTKSZ + SECRETBYTES)
>
> static void
> setup_stack(void)
> {
> + altstack = malloc(ALTSTACK_SIZE);
> +
> const stack_t sigstk = {
> .ss_sp = altstack,
> - .ss_size = sizeof(altstack),
> + .ss_size = ALTSTACK_SIZE
> };
>
> + ASSERT_NE(NULL, altstack);
> ASSERT_EQ(0, sigaltstack(&sigstk, NULL));
> }
>
> @@ -129,7 +134,7 @@ test_without_bzero()
> char buf[SECRETBYTES];
> assert_on_stack();
> populate_secret(buf, sizeof(buf));
> - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> ASSERT_NE(NULL, res);
> return (res);
> }
> @@ -140,7 +145,7 @@ test_with_bzero()
> char buf[SECRETBYTES];
> assert_on_stack();
> populate_secret(buf, sizeof(buf));
> - char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
> + char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
> ASSERT_NE(NULL, res);
> explicit_bzero(buf, sizeof(buf));
> return (res);
> @@ -183,14 +188,14 @@ main()
>* on the stack. This sanity checks that call_on_stack() and
>* populate_secret() work as intended.
>*/
> - memset(altstack, 0, sizeof(altstack));
> + memset(altstack, 0, ALTSTACK_SIZE);
> call_on_stack(do_test_without_bzero);
>
> /*
>* Now test with a call to explicit_bzero() and check that we
>* *don't* find any instances of the secret data.
>*/
> - memset(altstack, 0, sizeof(altstack));
> + memset(altstack, 0, ALTSTACK_SIZE);
> call_on_stack(do_test_with_bzero);
>
> return (0);
>
update explicit_bzero test to not assume SIGSTKSZ to be constant
In the next version of Linux glibc, SIGSTKSZ is defined at runtime if
source is built with _GNU_SOURCE. On LibreSSL-portable, this is set to
bring in asprintf/vasprintf, which causes the explicit_bzero test to
fail to compile since the size of SIGSTKSZ is no longer known at compile
time. This adjusts the test to treat SIGSTKSZ as a runtime variable.
See http://patches-tcwg.linaro.org/patch/48127/ and
https://github.com/libressl-portable/portable/issues/653 for the
LibreSSL build failure report on Fedora Rawhide.
ok?
Index: explicit_bzero.c
===
RCS file: /cvs/src/regress/lib/libc/explicit_bzero/explicit_bzero.c,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 explicit_bzero.c
--- explicit_bzero.c11 Jul 2014 01:10:35 - 1.6
+++ explicit_bzero.c23 Mar 2021 01:32:21 -
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
#include
@@ -36,16 +37,20 @@ enum {
SECRETBYTES = SECRETCOUNT * sizeof(secret)
};
-static char altstack[SIGSTKSZ + SECRETBYTES];
+static char *altstack;
+#define ALTSTACK_SIZE (SIGSTKSZ + SECRETBYTES)
static void
setup_stack(void)
{
+ altstack = malloc(ALTSTACK_SIZE);
+
const stack_t sigstk = {
.ss_sp = altstack,
- .ss_size = sizeof(altstack),
+ .ss_size = ALTSTACK_SIZE
};
+ ASSERT_NE(NULL, altstack);
ASSERT_EQ(0, sigaltstack(&sigstk, NULL));
}
@@ -129,7 +134,7 @@ test_without_bzero()
char buf[SECRETBYTES];
assert_on_stack();
populate_secret(buf, sizeof(buf));
- char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
+ char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
ASSERT_NE(NULL, res);
return (res);
}
@@ -140,7 +145,7 @@ test_with_bzero()
char buf[SECRETBYTES];
assert_on_stack();
populate_secret(buf, sizeof(buf));
- char *res = memmem(altstack, sizeof(altstack), buf, sizeof(buf));
+ char *res = memmem(altstack, ALTSTACK_SIZE, buf, sizeof(buf));
ASSERT_NE(NULL, res);
explicit_bzero(buf, sizeof(buf));
return (res);
@@ -183,14 +188,14 @@ main()
* on the stack. This sanity checks that call_on_stack() and
* populate_secret() work as intended.
*/
- memset(altstack, 0, sizeof(altstack));
+ memset(altstack, 0, ALTSTACK_SIZE);
call_on_stack(do_test_without_bzero);
/*
* Now test with a call to explicit_bzero() and check that we
* *don't* find any instances of the secret data.
*/
- memset(altstack, 0, sizeof(altstack));
+ memset(altstack, 0, ALTSTACK_SIZE);
call_on_stack(do_test_with_bzero);
return (0);
