Re: [PATCH] random: Fix crashes with sparse node ids

[Michael Ellerman]

Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> On Sat, Jul 30, 2016 at 7:23 AM, Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:
>>  #ifdef CONFIG_NUMA
>> -       pool = kmalloc(num_nodes * sizeof(void *),
>> +       pool = kmalloc(nr_node_ids * sizeof(void *),
>>                        GFP_KERNEL|__GFP_NOFAIL|__GFP_ZERO);
>>         for_each_online_node(i) {
>>                 crng = kmalloc_node(sizeof(struct crng_state),
>
> Ugh. Can we please also just change that kmalloc to kcalloc()? Get rid
> of the odd multiplication and the unusual GFP mask bit crud?
>
> And instead of using "sizeof(void *)", just use the pool entry size,
> ie "sizeof(*pool)". Yes, we have other places where we depend on void
> pointers having the same size as others, but it's the RightThing(tm)
> to do anyway, and it makes more sense when you grep things ("Oh, we're
> allocating 'nr_node_id' copes of *pool entries" even without knowing
> what type is behind the "pool" pointer).
>
> IOW, can you confirm that you could just use
>
>         pool = kcalloc(nr_node_ids, sizeof(*pool), GFP_KERNEL);
>
> instead? I'd much rather apply that patch.

Dropping NOFAIL means we need to handle allocation failures, which makes
the patch a bit bigger, and less of a pure fix.

Here's a separate patch to do those cleanups, which you can squash with
the first if you prefer.

I did test the allocation failure case for both the whole pool and
individual nodes.

cheers