linux-kernel - Re: [PATCH][next] drm/nouveau: chan: 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <25518061-7b9d-4a43-be53-c5aee80c0f1d@embeddedor.com>
Date: Fri, 11 Apr 2025 01:26:15 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Kees Cook <kees@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 Lyude Paul <lyude@...hat.com>, Danilo Krummrich <dakr@...nel.org>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH][next] drm/nouveau: chan: Avoid
 -Wflex-array-member-not-at-end warnings



On 08/04/25 17:40, Kees Cook wrote:
> On Mon, Apr 07, 2025 at 05:35:47PM -0600, Gustavo A. R. Silva wrote:
>> [..]
>>
>>>>>> -	struct {
>>>>>> -		struct nvif_chan_v0 chan;
>>>>>> -		char name[TASK_COMM_LEN+16];
>>>>>> -	} args;
>>>>>> +	DEFINE_RAW_FLEX(struct nvif_chan_v0, args, name, TASK_COMM_LEN + 16);
>>>>>>     	struct nvif_device *device = &cli->device;
>>>>>>     	struct nouveau_channel *chan;
>>>>>>     	const u64 plength = 0x10000;
>>>>>> @@ -298,28 +295,28 @@ nouveau_channel_ctor(struct nouveau_cli *cli, bool priv, u64 runm,
>>>>>>     		return ret;
>>>>>>     	/* create channel object */
>>>>>> -	args.chan.version = 0;
>>>>>> -	args.chan.namelen = sizeof(args.name);
>>>>>> -	args.chan.runlist = __ffs64(runm);
>>>>>> -	args.chan.runq = 0;
>>>>>> -	args.chan.priv = priv;
>>>>>> -	args.chan.devm = BIT(0);
>>>>>> +	args->version = 0;
>>>>>> +	args->namelen = __struct_size(args) - sizeof(*args);
>>>>>
>>>>> Does __struct_size(args->name) work here (and later)?
>>>>
>>>> Why not?
>>>
>>> Uhm, I'm genuinely curious. I *think* it will work, but because it's
>>> within the struct, not outside of it, I'm unclear if it'll DTRT for
>>> finding the size (since __builtin_object_size() can be touchy).
>>>
>>>> I mean, this should be equivalent to `TASK_COMM_LEN+16`, I could
>>>> use the latter if people prefer it (see my comments below).
>>>
>>> Right, it should be the same. I think __struct_size(args->name) would be
>>> much more readable ... if it works. :)
>>
>> OK, I'll double check this.
> 
> Ah-ha, yes, I'm already testing this with KUnit:
> 
> struct bar {
>          int a;
>          u32 counter;
>          s16 array[];
> };
> ...
>          DEFINE_RAW_FLEX(struct bar, two, array, 2);
> 	...
>          KUNIT_EXPECT_EQ(test, sizeof(*two), sizeof(struct bar));
>          KUNIT_EXPECT_EQ(test, __struct_size(two), sizeof(struct bar) + 2 * sizeof(s16));
>          KUNIT_EXPECT_EQ(test, __member_size(two), sizeof(struct bar) + 2 * sizeof(s16));
>          KUNIT_EXPECT_EQ(test, __struct_size(two->array), 2 * sizeof(s16));
>          KUNIT_EXPECT_EQ(test, __member_size(two->array), 2 * sizeof(s16));

Nice! I was taking a look at this, and now I want to use __member_size(p->array)
instead of __struct_size(p->array). ^.^

> 
> 
>> I really don't want to condition -Wflex-array-member-not-at-end patches
>> on counted_by patches, for now.
> 
> Fair enough. :) One thing at a time is wise!
> 

\o/

--
Gustavo