Avoiding stack buffer clear being optimised out

Avoiding stack buffer clear being optimised out

[Jonny Grant]

[-- Attachment #1: Type: text/plain, Size: 1356 bytes --]

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)

    // 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);
    void * addr = memset(key, 0, 8);
    //printf("encrypting with super secret key: %s\n", key);

    if(addr) return 1;
    else return 0;
}

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();
    printf("ret:%d\n", ret);
    /* notify that we're done */
    log_completion();
    return ret;
}

Jonny

Re: Avoiding stack buffer clear being optimised out

[Jonathan Wakely]

On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org> 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.

Re: Avoiding stack buffer clear being optimised out

[Jonathan Wakely]

On Wed, 30 Nov 2022 at 17:40, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
> On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org> 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)

Oops, I meant to change that to return void, because you don't need to
jump through hoops checking its return value to ensure the side
effects aren't optimized out.

> {
>     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.

Re: Avoiding stack buffer clear being optimised out

[Jonny Grant]

On 30/11/2022 17:41, Jonathan Wakely wrote:
> On Wed, 30 Nov 2022 at 17:40, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>>
>> On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org> 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)
> 
> Oops, I meant to change that to return void, because you don't need to
> jump through hoops checking its return value to ensure the side
> effects aren't optimized out.
> 
>> {
>>     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.

Thank you Jonathan and David for your replies.

That "noipa" looks to have sorted this issue

https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

That page also suggests "noinline" attribute which seems to suggest I'd need to add asm (""); in each wrapper of memset()


I'd much rather have memset_s - Jonathan, do you think GCC could add some built-in functions for memset_s ?     __builtin_memset_s() would be great.

There are quite a few similar ones that should be easy to add based on existing
 (memcpy_s, memmove_s, strcpy_s, strncpy_s, strcat_s, strncat_s, strtok_s, memset_s, strerror_s, strerrorlen_s, strnlen_s).

I did speak to someone at LLVM who was considering adding built-ins to clang.

Kind regards
Jonny

Re: Avoiding stack buffer clear being optimised out

[Jonathan Wakely]

On Thu, 1 Dec 2022 at 10:44, Jonny Grant wrote:
> Thank you Jonathan and David for your replies.
>
> That "noipa" looks to have sorted this issue
>
> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
>
> That page also suggests "noinline" attribute which seems to suggest I'd need to add asm (""); in each wrapper of memset()

I already used the noinline attribute in my example above.

> I'd much rather have memset_s - Jonathan, do you think GCC could add some built-in functions for memset_s ?     __builtin_memset_s() would be great.

No.

But C2x adds a memset_explicit function that does what you want, so
that should arrive in glibc soonish.
I thought it had been added, but was searching the C2x draft for
"memset_secure" and other incorrect names.

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2631.htm was the
proposal adding it.

>
> There are quite a few similar ones that should be easy to add based on existing
>  (memcpy_s, memmove_s, strcpy_s, strncpy_s, strcat_s, strncat_s, strtok_s, memset_s, strerror_s, strerrorlen_s, strnlen_s).

They're not good APIs. See
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm and
https://sourceware.org/legacy-ml/libc-help/2018-01/msg00007.html


> I did speak to someone at LLVM who was considering adding built-ins to clang.
>
> Kind regards
> Jonny
>
>

Re: Avoiding stack buffer clear being optimised out

[Jonathan Wakely]

On Thu, 1 Dec 2022 at 11:31, Jonathan Wakely wrote:
>
> On Thu, 1 Dec 2022 at 10:44, Jonny Grant wrote:
> > Thank you Jonathan and David for your replies.
> >
> > That "noipa" looks to have sorted this issue
> >
> > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
> >
> > That page also suggests "noinline" attribute which seems to suggest I'd need to add asm (""); in each wrapper of memset()
>
> I already used the noinline attribute in my example above.
>
> > I'd much rather have memset_s - Jonathan, do you think GCC could add some built-in functions for memset_s ?     __builtin_memset_s() would be great.
>
> No.
>
> But C2x adds a memset_explicit function that does what you want, so
> that should arrive in glibc soonish.
> I thought it had been added, but was searching the C2x draft for
> "memset_secure" and other incorrect names.
>
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2631.htm was the
> proposal adding it.

Ah, and that proposal links to an example implementation which uses
glibc's explicit_bzero which already exists (since glibc 2.25, and
also in some BSDs) and does what you want.
https://github.com/ojeda/secure_clear/blob/master/example-implementation/secure_clear.h

Re: Avoiding stack buffer clear being optimised out

[Jonny Grant]

On 01/12/2022 11:31, Jonathan Wakely wrote:
> On Thu, 1 Dec 2022 at 10:44, Jonny Grant wrote:
>> Thank you Jonathan and David for your replies.
>>
>> That "noipa" looks to have sorted this issue
>>
>> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
>>
>> That page also suggests "noinline" attribute which seems to suggest I'd need to add asm (""); in each wrapper of memset()
> 
> I already used the noinline attribute in my example above.
> 
>> I'd much rather have memset_s - Jonathan, do you think GCC could add some built-in functions for memset_s ?     __builtin_memset_s() would be great.
> 
> No.
> 
> But C2x adds a memset_explicit function that does what you want, so
> that should arrive in glibc soonish.
> I thought it had been added, but was searching the C2x draft for
> "memset_secure" and other incorrect names.
> 
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2631.htm was the
> proposal adding it.

Well, at least it is in the standard. Although now we have yet another function name doing similar.
Might have been simpler to add explicit_bzero to the standard.

I can see this has been discussed for two decades
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=8537

So it will be like this:
void *memset_explicit(void *s, int c, size_t n);

I imagine it won't check the pointer is non-NULL. memset_s did do that. Seems pointless to return the pointer.

Hopefully no one will use these other variations people have implemented with different parameter types.
https://github.com/gsbabil/memset_explicit/blob/master/memset_explicit.h

>>
>> There are quite a few similar ones that should be easy to add based on existing
>>  (memcpy_s, memmove_s, strcpy_s, strncpy_s, strcat_s, strncat_s, strtok_s, memset_s, strerror_s, strerrorlen_s, strnlen_s).
> 
> They're not good APIs. See
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm and
> https://sourceware.org/legacy-ml/libc-help/2018-01/msg00007.html

Yes, I know there is a lot of disapproval of Annex K. although I don't feel those trivial memset_s style are. I liked them as they check for non-NULL and return a handy error code.

Regards, Jonny

Re: Avoiding stack buffer clear being optimised out

[Jonny Grant]

On 30/11/2022 17:40, Jonathan Wakely wrote:
> On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org> 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.

If I modify the code to check the buffer contains key[0] as nul byte, it still shows as 0, 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;
}

Re: Avoiding stack buffer clear being optimised out

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 3131 bytes --]

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

>
>
> On 30/11/2022 17:40, Jonathan Wakely wrote:
> > On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org> 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;
> }
>
>
>

Re: Avoiding stack buffer clear being optimised out

[Jonny Grant]

On 13/12/2022 20:31, Jonathan Wakely wrote:
> 
> 
> On Tue, 13 Dec 2022, 20:12 Jonny Grant, <jg@jguk.org <mailto:jg@jguk.org>> wrote:
> 
> 
> 
>     On 30/11/2022 17:40, Jonathan Wakely wrote:
>     > On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org <mailto:jg@jguk.org>> 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/ <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 <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;

ok, that is what I thought, it seems to know how the internals of memset() work. Maybe because it's a builtin?

> 
>     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.

That makes sense. So if I wrote a assembler implementation I called memset_asm() I imagine gcc wouldn't be able to understand that it set those bytes to zero? so it wouldn't remove my memset_asm() call.

Regards, Jonny

Re: Avoiding stack buffer clear being optimised out

[Jonathan Wakely]

On Tue, 13 Dec 2022 at 22:07, Jonny Grant <jg@jguk.org> wrote:
>
>
>
> On 13/12/2022 20:31, Jonathan Wakely wrote:
> >
> >
> > On Tue, 13 Dec 2022, 20:12 Jonny Grant, <jg@jguk.org <mailto:jg@jguk.org>> wrote:
> >
> >
> >
> >     On 30/11/2022 17:40, Jonathan Wakely wrote:
> >     > On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org <mailto:jg@jguk.org>> 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/ <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 <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;
>
> ok, that is what I thought, it seems to know how the internals of memset() work. Maybe because it's a builtin?

It doesn't need to know anything about the internals, only the
observable behaviour, which is entirely defined by the C standard.


>
> >
> >     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.
>
> That makes sense. So if I wrote a assembler implementation I called memset_asm() I imagine gcc wouldn't be able to understand that it set those bytes to zero? so it wouldn't remove my memset_asm() call.
>
> Regards, Jonny

Re: Avoiding stack buffer clear being optimised out

[Jonny Grant]

On 13/12/2022 23:13, Jonathan Wakely wrote:
> On Tue, 13 Dec 2022 at 22:07, Jonny Grant <jg@jguk.org> wrote:
>>
>>
>>
>> On 13/12/2022 20:31, Jonathan Wakely wrote:
>>>
>>>
>>> On Tue, 13 Dec 2022, 20:12 Jonny Grant, <jg@jguk.org <mailto:jg@jguk.org>> wrote:
>>>
>>>
>>>
>>>     On 30/11/2022 17:40, Jonathan Wakely wrote:
>>>     > On Wed, 30 Nov 2022 at 16:27, Jonny Grant <jg@jguk.org <mailto:jg@jguk.org>> 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/ <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 <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;
>>
>> ok, that is what I thought, it seems to know how the internals of memset() work. Maybe because it's a builtin?
> 
> It doesn't need to know anything about the internals, only the
> observable behaviour, which is entirely defined by the C standard

ok thank you for clarifying.

Jonny

Re: Avoiding stack buffer clear being optimised out

[Jonny Grant]

Probably other functions be "compiled out" like memset() is, would memcpy() be compiled out?
Jonny

Re: Avoiding stack buffer clear being optimised out

[Jonathan Wakely]

On Tue, 24 Jan 2023 at 13:52, Jonny Grant <jg@jguk.org> wrote:
>
> Probably other functions be "compiled out" like memset() is, would memcpy() be compiled out?

The compiler can do anything which does not change the observable side
effects of a valid program.

https://en.cppreference.com/w/cpp/language/as_if

So yes, the compiler can omit a memcpy that doesn't do anything. Or
any other function call that doesn't do anything.

Re: Avoiding stack buffer clear being optimised out

[David Brown]

On 30/11/2022 17:26, Jonny Grant 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)
> 
>      // 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);
>      void * addr = memset(key, 0, 8);
>      //printf("encrypting with super secret key: %s\n", key);
> 
>      if(addr) return 1;
>      else return 0;
> }
> 
> 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();
>      printf("ret:%d\n", ret);
>      /* notify that we're done */
>      log_completion();
>      return ret;
> }
> 
> Jonny
> 

If you are happy with gcc extensions, inline assembly can give you what 
you want.  It is even highly portable to different processor targets 
(and to clang), because it has no actual assembly code!

<https://godbolt.org/z/6nfY5o89a>

The simplest "sledgehammer" technique here is a memory barrier using a 
memory clobber after the "memset" :

	asm volatile("" ::: "memory");

This tells the compiler to output the instruction sequence "", and that 
it might read or write to anything in memory in unexpected ways.  So the 
compiler will flush out any "uncommitted" writes to memory before 
"executing" this code.  And it will re-read anything that had been 
cached in registers.  So it is quite general, but can have a significant 
performance impact on some kinds of code.  (Still, it is much less 
dramatic than a real memory barrier or memory fence instruction.)


A more targeted approach is to follow the "memset" with :

	asm volatile("" :: "m" (key) : );

This again has an empty set of assembly instructions.  But it tells the 
compiler that those instructions need the value of "key", available in 
memory.  So the "memset" must again be completed before "executing" the 
assembly instructions.  However, nothing else need be touched.


Of course, none of this helps if the compiler has made extra copies of 
the key, or left copies on the stack from other calls.


(I hope that Jonathan or other gcc experts can correct me if I am 
assuming too much from such inline assembly.  I have used this kind of 
construct on a number of occasions, but I have always checked the 
generated assembly afterwards.)