linux-hardening - Re: [PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end warnings

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 for Android: free password hash cracker in your pocket

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

Message-ID: <pw6lpxztxeg2sjwk5mheqlv4scl3zzhhpegnawf5umap6rxe4y@yckkngbinoy2>
Date: Mon, 15 Sep 2025 11:36:40 +0200
From: Uwe Kleine-König <ukleinek@...nel.org>
To: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>, 
	Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>, linux-pwm@...r.kernel.org, 
	chrome-platform@...ts.linux.dev, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] pwm: cros-ec: Avoid -Wflex-array-member-not-at-end
 warnings

Hello,

On Thu, Aug 14, 2025 at 04:08:57PM +0200, Uwe Kleine-König wrote:
> On Thu, Aug 14, 2025 at 08:48:23PM +0900, Gustavo A. R. Silva wrote:
> > 
> > > diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> > > index dab49e2ec8c0..8ca9df87a523 100644
> > > --- a/include/linux/stddef.h
> > > +++ b/include/linux/stddef.h
> > > @@ -108,7 +108,7 @@ enum {
> > >   	union {									\
> > >   		TYPE NAME;							\
> > >   		struct {							\
> > > -			unsigned char __offset_to_##FAM[offsetof(TYPE, FAM)];	\
> > > +			unsigned char __offset_to_##FAM[sizeof(TYPE)];		\
> > >   			MEMBERS							\
> > >   		};								\
> > >   	}
> > > 
> > > which only leaves one usage of FAM in the name of the padding struct
> > > member. I'm sure someone is able to come up with something nice here to
> > > get rid of FAM completely or point out what I'm missing.
> > 
> > Flexible structures (structs that contain a FAM) may have trailing padding.
> > Under that scenario sizeof(TYPE) causes the overlay between FAM and MEMBERS
> > to be misaligned.
> 
> That sounds wrong to me; are you sure? In that case allocating space for
> such a struct using
> 
> 	struct mystruct {
> 		unsigned short len;
> 		unsigned int array[];
> 	};
> 
> 	s = malloc(sizeof(struct mystruct) + n * sizeof(unsigned int));
> 
> wouldn't do the right thing. 
> 
> I found in the net (e.g.
> https://rgambord.github.io/c99-doc/sections/6/7/2/1/index.html):
> 
> 	In most situations, the flexible array member is ignored. In
> 	particular, the size of the structure is as if the flexible
> 	array member were omitted except that it may have more trailing
> 	padding than the omission would imply.
> 
> So I'd claim that sizeof does work here as intended.
> 
> gcc here also behaves fine:
> 
> 	uwe@...rus:~$ cat test.c
> 	#include <stdio.h>
> 
> 	struct mystruct {
> 		unsigned short len;
> 		unsigned int array[];
> 	};
> 
> 	struct mystruct2 {
> 		unsigned short len;
> 	};
> 
> 	int main()
> 	{
> 		printf("sizeof(struct mystruct) = %zu\n", sizeof(struct mystruct));
> 		printf("sizeof(struct mystruct2) = %zu\n", sizeof(struct mystruct2));
> 		return 0;
> 	}
> 
> 	uwe@...rus:~$ make test
> 	cc    -c -o test.o test.c
> 	cc   test.o   -o test
> 
> 	uwe@...rus:~$ ./test
> 	sizeof(struct mystruct) = 4
> 	sizeof(struct mystruct2) = 2

My claim is wrong, while sizeof() never gives a value that is too small,
it might be too big. E.g. for

	struct mystruct {
		unsigned short a;
		unsigned char b;
		unsigned char c[];
	};

there is sizeof(mystruct) = 4, but c starts at offset 3.

Anyhow, I applied the original patch now to

https://git.kernel.org/pub/scm/linux/kernel/git/ukleinek/linux.git pwm/for-next

; the discussion here was somewhat orthogonal anyhow.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)