linux-kernel - RE: struct_size() using sizeof() vs offsetof()

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <55bb7e4f633340db9a9c013b91afedd6@AcuMS.aculab.com>
Date:   Mon, 21 Aug 2023 08:16:17 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Alejandro Colomar' <alx@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>
CC:     LKML <linux-kernel@...r.kernel.org>
Subject: RE: struct_size() using sizeof() vs offsetof()

From: Alejandro Colomar
> Sent: Thursday, August 17, 2023 1:23 AM
> 
> Hi Kees, Gustavo,
> 
> I've been discussing with a friend about the appropriateness of sizeof()
> vs offsetof() for calculating the size of a structure with a flexible
> array member (FAM).
> 
> After reading Jens Gustedt's blog post about it[1], we tried some tests,
> and we got some interesting results that discouraged me from using sizeof().
> See below.
> 
> But then, said friend pointed to me that the kernel uses sizeof() in
> struct_size(), and we wondered why you would have chosen it.  It's safe
> as long as you _know_ that there's no padding, or that the alignment of
> the FAM is as large as the padding (which you probably know in the kernel),
> but it seems safer to use
> 
> 	MAX(sizeof(s), offsetof(s, fam) + sizeof_member(s, fam) * count)
> 
> The thing is, if there's any trailing padding in the struct, the FAM may
> overlap the padding, and the calculation with sizeof() will waste a few
> bytes, and if misused to get the location of the FAM, the problem will be
> bigger, as you'll get a wrong location.
> 
> So, I just wanted to pry what and especially why the kernel chose to prefer
> a simple sizeof().
> 
> Cheers,
> Alex
> 
> ---
.....
> 	strcpy(s->fam, "Hello, sizeof!");
> 	p = (char *) s + sizeof(struct s);
> 	puts(p);

Why on earth would you expect the above to do anything sensible?

It is a shame you can just use offsetof(type, member[count + 1]).
That is fine for constants, but the C language requires offsetof()
to be a compile-time constant - I can't help feeling the standards
body didn't consider non-constant array offsets.
(The compiler for a well known OS won't compile that (or anything
that looks like it) even for a constant array subscript!)

The actual problem with using offsetof() is that you might end
up with something smaller than the structure size.
(When the variable sized array is smaller than the padding.)

While max() will generate a constant for constant input, it
will be a real compare for non-constant input.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)