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  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

Powered by blists - more mailing lists