Re: [PATCH 6/6] Add test for explicit_bzero

[Bastien ROUCARIES]

Yes, it does.

But I will prefer that you push first the test (I have not written
myself and it need comments that are outside my english language
skills).

So could you push a test ?

I will redo my serie above your test

On Sun, Apr 12, 2020 at 1:22 PM Bruno Haible <address@hidden> wrote:
>
> Hi Bastien,
>
> > Simple test
>
> A module description file modules/explicit_bzero-tests is needed as well.
>
> Then you can test the changed implementation through
> 1.
>   ./gnulib-tool --create-testdir --dir=/tmp/testdir --single-configure 
> explicit_bzero
> 2. transport the test dir to a different machine,
> 3. test it there.
>
> > @@ -0,0 +1,34 @@
> > +/* Test of asinl() function.
>
> This first line is a copy&paste.
>
> > +   Copyright (C) 2010-2020 Free Software Foundation, Inc.
>
> You may have started with a file written in 2010. But since no
> copyrightable (= significant) code is left over from it, the copyright
> year range should just be 2020.
>
> > +#include <config.h>
> > +
> > +#include <string.h>
> > +
> > +#include "macros.h"
>
> OK.
>
> > +char hello[] = "hello";
> > +
> > +int
> > +main ()
> > +{
> > +  explicit_bzero(hello,sizeof(hello));
> > +  ASSERT (hello[0]==0);
>
> The unit test should have two purposes:
>   1) Verify that the function is well defined (no link error) and
>      runs (does not crash).
>   2) Verify that the compiler does not eliminate it.
>
> Re 1), your proposed test does it.
>
> Re 2): No compiler would optimize away a memset on a global variable.
> Compilers are most tempted to optimize for an array allocated on the stack,
> at a point where the array is going out of scope. Like this:
>
>    void
>    do_secret_stuff (void)
>    {
>      char pwd[6];
>      memset (pwd, "secret", 6);
>      explicit_bzero (pwd, 6);
>    }
>
> But now, how to check that the array is cleared, since it's already out of
> scope? I would try it like this:
>
>    int
>    do_secret_stuff (volatile int pass)
>    {
>      char pwd[6];
>      if (pass == 1)
>        {
>          memset (pwd, "secret", 6);
>          explicit_bzero (pwd, 6);
>          return 0;
>        }
>      else
>        {
>          static char zero [6] = { 0 };
>          return memcmp (pwd, zero, 6) != 0;
>        }
>    }
>
> The caller will do
>    do_secret_stuff (1);
>    do_secret_stuff (2);
>
> But there's still a small probability of an interrupt that clobbers the
> stack between the two calls. Therefore I would put this in a loop:
>
>    int count = 0;
>    int repeat = 1000;
>    for (; repeat > 0; repeat--)
>      {
>        do_secret_stuff (1);
>        count += do_secret_stuff (2);
>      }
>    ASSERT (count < 50);
>
> The idea is that if, in the vast majority of cases, do_secret_stuff (2)
> returned 0, explicit_bzero worked as expected. If the compiler optimized
> it away, you would get count > 950.
>
> Does that sound sensible? And does it work (i.e. if you use a plain
> memset instead of explicit_bzero, does the test fail)?
>
> Bruno
>