On Fri, Jul 31, 2020 at 11:36:04AM -0300, Jason Gunthorpe wrote:

On Fri, Jul 31, 2020 at 04:21:48PM +0200, Greg Kroah-Hartman wrote:

quoted

quoted

The spec was updated in C11 to require zero'ing padding when doing
partial initialization of aggregates (eg = {})

"""if it is an aggregate, every member is initialized (recursively)
according to these rules, and any padding is initialized to zero
bits;"""

But then why does the compilers not do this?

Do you have an example?

At the moment, no, but we have had them in the past due to security
issues we have had to fix for this.

quoted

quoted

Considering we have thousands of aggregate initializers it
seems likely to me Linux also requires a compiler with this C11
behavior to operate correctly.

Note that this is not an "operate correctly" thing, it is a "zero out
stale data in structure paddings so that data will not leak to
userspace" thing.

Yes, not being insecure is "operate correctly", IMHO :)
 

quoted

quoted

Does this patch actually fix anything? My compiler generates identical
assembly code in either case.

What compiler version?

I tried clang 10 and gcc 9.3 for x86-64.

#include <string.h>

void test(void *out)
{
	struct rds_rdma_notify {
		unsigned long user_token;
		unsigned int status;
	} foo = {};
	memcpy(out, &foo, sizeof(foo));
}

$ gcc -mno-sse2 -O2 -Wall -std=c99 t.c -S

test:
	endbr64
	movq	$0, (%rdi)
	movq	$0, 8(%rdi)
	ret

Just did this same test with gcc 4.4 and it also gave the same output..

Made it more complex with this:

	struct rds_rdma_notify {
		unsigned long user_token;
		unsigned char status;
		unsigned long user_token1;
		unsigned char status1;
		unsigned long user_token2;
		unsigned char status2;
		unsigned long user_token3;
		unsigned char status3;
		unsigned long user_token4;
		unsigned char status4;
	} foo;

And still got the same assembly vs memset on gcc 4.4.

I tried for a bit and didn't find a way to get even old gcc 4.4 to not
initialize the holes.

Odd, so it is just the "= {0};" that does not zero out the holes?

thanks,

greg k-h