On Fri, Mar 26, 2021 at 1:56 PM Alexander Bluhm <[email protected]> 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 -0000 1.6
> > +++ explicit_bzero.c 23 Mar 2021 01:32:21 -0000
> > @@ -18,6 +18,7 @@
> > #include <assert.h>
> > #include <errno.h>
> > #include <signal.h>
> > +#include <stdlib.h>
> > #include <string.h>
> > #include <unistd.h>
> >
> > @@ -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);
