On Wed, 2020-05-20 at 10:19 -0700, Kees Cook wrote:
> On Mon, May 18, 2020 at 12:09:16PM -0700, Joe Perches wrote:
> > On Mon, 2020-05-18 at 14:01 -0500, Gustavo A. R. Silva wrote:
> > > The current codebase makes use of one-element arrays in the following
> > > form:
> > >
> > > struct something {
> > > int length;
> > > u8 data[1];
> > > };
> > []
> > > This issue has been out there since 2009.
> > > This issue was found with the help of Coccinelle and fixed _manually_.
> > []
> > > diff --git a/arch/x86/platform/uv/uv_time.c b/arch/x86/platform/uv/uv_time.c
[]
> > > @@ -156,9 +156,8 @@ static __init int uv_rtc_allocate_timers(void)
> > > struct uv_rtc_timer_head *head = blade_info[bid];
> > >
> > > if (!head) {
> > > - head = kmalloc_node(sizeof(struct uv_rtc_timer_head) +
> > > - (uv_blade_nr_possible_cpus(bid) *
> > > - 2 * sizeof(u64)),
> > > + head = kmalloc_node(struct_size(head, cpu,
> > > + uv_blade_nr_possible_cpus(bid)),
> >
> > It's probably safer to use kzalloc_node here as well.
>
> Hm, I think it's not actually needed here.
Right. Turns out it's not needed.
> All three members are
> immediately initialized and it doesn't look to ever be copied to
> userspace.
It's more that the reader doesn't have to lookup the
struct to know all the members are initialized.
It's also not a fast path so any extra time to zero
is not significant.
> > > GFP_KERNEL, nid);
> > > if (!head) {
> > > uv_rtc_deallocate_timers();
>
> FWIW, I think this change is good as-is. Always nice to get back a
> little memory. ;)
Saving space, 0 to 4 bytes at a time,
depending on the heap sizes...
Using the struct_size is clearer though, so that's good too.
cheers, Joe
