two bogus patches arising from CVE-2019-12381

two bogus patches arising from CVE-2019-12381

[Charles Bryant]

I believe two patches from last year are mistaken. They are:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=95baa60a0da80a0143e3ddd4d3725758b4513825

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=425aa0e1d01513437668fa3d4a971168bbaa8515

Both of these make a function return immediately with ENOMEM if a kalloc()
fails.  However in each case the function already correctly handled
allocation failure later on. Furthermore, by making them exit early
on allocation failure, it (very slightly) makes them worse as in some
cases they might have correctly returned EADDRINUSE and not needed the
allocated memory.

I think, therefore, that these changes should be reverted.
-- 
Charles Bryant

Re: two bogus patches arising from CVE-2019-12381

[Florian Westphal]

Charles Bryant <ch.4g7vxy-nbkl8p@chch.co.uk> wrote:
> I believe two patches from last year are mistaken. They are:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=95baa60a0da80a0143e3ddd4d3725758b4513825
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=425aa0e1d01513437668fa3d4a971168bbaa8515
> 
> Both of these make a function return immediately with ENOMEM if a kalloc()
> fails.  However in each case the function already correctly handled
> allocation failure later on. Furthermore, by making them exit early
> on allocation failure, it (very slightly) makes them worse as in some
> cases they might have correctly returned EADDRINUSE and not needed the
> allocated memory.
> 
> I think, therefore, that these changes should be reverted.

Both fixes are useless, as you explained above.

But they do not matter.  When GFP_KERNEL allocations fail the entire
system is screwed anyway.

So instead of revert, I would suggest that you wait until net-next
reopens, then:

For the first commit, send a patch that reverts, but also add a comment
that explains the error is handled below the loop.

For the second commit, send a patch that moves the allocation to where its
needed -- the spinlock was converted to a mutex so there is no need
for this ahead-of-time allocation anymore.