Re: Avoiding stack buffer clear being optimised out

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 13 Dec 2022, 20:12 Jonny Grant, <jg@xxxxxxxx> wrote:

>
>
> On 30/11/2022 17:40, Jonathan Wakely wrote:
> > On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@xxxxxxxx> wrote:
> >>
> >> Hello
> >>
> >> Does GCC have a clear way to avoid memset being compiled out by
> optimiser?
> >>
> >> This article came up, so I combined the broken.c with GCC
> >> gcc -Wall -O2 -o broken broken.c
> >>
> >> Note, I've been using gcc for many years, I'm not looking for just tips
> how to compile code. I only want to discuss this optimiser issue :-)
> >>
> >>
> https://blog.cloudflare.com/the-linux-kernel-key-retention-service-and-why-you-should-use-it-in-your-next-application/
> >>
> >> If I modify to clear the buffer, it gets removed by the compiler
> >>
> >> The only way I could get it to not remove the memset is by adding
> another printf, (propagating a return code after checking memset wasn't
> enough)
> >
> > This is simpler and works for me, but I'm not sure if it's guaranteed
> > to always work:
> >
> > __attribute__((noinline,noipa))
> > void wipe(void* p, size_t n)
> > {
> >   memset(p, 0, n);
> > }
> >
> > static int encrypt(void)
> > {
> >     uint8_t key[] = "hunter2";
> >     printf("encrypting with super secret key: %s\n", key);
> >     wipe(key, 8);
> > }
> >
> > There is discussion of alternatives in
> > https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1358.pdf (starting
> > on page 6).
> >
> > The memset_s function was added to C in Annex K, but most
> > implementations of the C library do not support Annex K.
>
>
> Jonathan
>
> I wonder if you know how GCC decides to remove the memset? In the
> following example, the memset even changes the bytes on the stack, which
> are then not changed later on in the program, rather strange.
>

No it doesn't.

The observable behaviour of the function is to write zero to several bytes,
then check if one of them was zero. The compiler doesn't need to write
anything or read anything to produce that behaviour, the read can never
*not* read a zero there, so the compiler doesn't bother to write anything
*or* read anything. It optimizes the whole function to simply:

puts("encrypting with super secret key: hunter2");
return 0;


If I modify the code to check the buffer contains key[0] as nul byte, it
> still shows as 0,


You asked whether something just set to zero is equal to zero. That's
redundant, of course it is. There is no need to waste CPU cycles on that.


but then the stack is still readable
>
> // gcc -Wall -O2 -o broken broken.c
>
> #include <stdio.h>
> #include <stdint.h>
> #include <string.h>
>
> static int encrypt(void)
> {
>     uint8_t key[] = "hunter2";
>     printf("encrypting with super secret key: %s\n", key);
>     memset(key, 0, 8);
>     if(key[0] == '\0') return 0;
>     else return 1;
> }
>
> static void log_completion(void)
> {
>     /* oh no, we forgot to init the msg */
>     char msg[8];
>     printf("not important, just fyi: %s\n", msg);
> }
>
> int main(void)
> {
>     int ret = encrypt();
>     /* notify that we're done */
>     log_completion();
>     printf("ret: %d\n", ret);
>     return ret;
> }
>
>
>



[Index of Archives]     [Linux C Programming]     [Linux Kernel]     [eCos]     [Fedora Development]     [Fedora Announce]     [Autoconf]     [The DWARVES Debugging Tools]     [Yosemite Campsites]     [Yosemite News]     [Linux GCC]

  Powered by Linux