linux-hardening - Re: [PATCH -next, v2] sched: Use struct_size() helper in task_numa

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]

Date:   Tue, 11 Jan 2022 10:14:25 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Xiu Jianfeng <xiujianfeng@...wei.com>, mingo@...hat.com,
        juri.lelli@...hat.com, vincent.guittot@...aro.org,
        dietmar.eggemann@....com, bsegall@...gle.com, mgorman@...e.de,
        bristot@...hat.com, gustavoars@...nel.org,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH -next, v2] sched: Use struct_size() helper in
 task_numa_group()

On Tue, 11 Jan 2022 12:30:42 +0100
Peter Zijlstra <peterz@...radead.org> wrote:

> > > >  	if (unlikely(!deref_curr_numa_group(p))) {
> > > > -		unsigned int size = sizeof(struct numa_group) +
> > > > -				    NR_NUMA_HINT_FAULT_STATS *
> > > > -				    nr_node_ids * sizeof(unsigned long);
> > > > +		unsigned int size = struct_size(grp, faults,
> > > > +						NR_NUMA_HINT_FAULT_STATS * nr_node_ids);    
> > > 
> > > Again, why?! The old code was perfectly readable, this, not so much.  
> > 
> > Because it is unsafe,  
> 
> Unsafe how? Changelog doesn't mention anything, nor do you. In fact,
> Changelog says there is no functional change, which makes me hate the
> thing for obscuring something that was simple.

If for some reason faults changes in size, the original code must be
updated whereas the new code is robust enough to not need changing.

> 
> > And to be honest, the new change is a lot easier to read than the original
> > code.  
> 
> I find it the other way around, because now I need to find and untangle
> the unholy mess that is struct_size(), whereas currently it is trivial
> C.

It's a C hack and far from trivial. Maybe to you as you are use to
these hacks. But seriously, this is not something the average C coder
is use to, as variable length structures are rather unique to the
kernel.

Note that struct_size() is commonly used in the kernel. Better start
getting use to it ;-)

-- Steve