From: Patrick Eigensatz <[email protected]>
After allocating the spare nexthop group it should be tested for kzalloc()
returning NULL, instead the already used nexthop group (which cannot be
NULL at this point) had been tested so far.
Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL.
Coverity-id: 1463885
Reported-by: Coverity <[email protected]>
Signed-off-by: Patrick Eigensatz <[email protected]>
---
net/ipv4/nexthop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 563f71bcb2d7..cb9412cd5e4b 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -1118,10 +1118,10 @@ static struct nexthop *nexthop_create_group(struct net *net,
/* spare group used for removals */
nhg->spare = nexthop_grp_alloc(num_nh);
- if (!nhg) {
+ if (!nhg->spare) {
kfree(nhg);
kfree(nh);
- return NULL;
+ return ERR_PTR(-ENOMEM);
}
nhg->spare->spare = nhg;
--
2.26.2
On 01/06/2020 14:12, [email protected] wrote:
> From: Patrick Eigensatz <[email protected]>
>
> After allocating the spare nexthop group it should be tested for kzalloc()
> returning NULL, instead the already used nexthop group (which cannot be
> NULL at this point) had been tested so far.
>
> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL.
>
> Coverity-id: 1463885
> Reported-by: Coverity <[email protected]>
> Signed-off-by: Patrick Eigensatz <[email protected]>
> ---
> net/ipv4/nexthop.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index 563f71bcb2d7..cb9412cd5e4b 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
> @@ -1118,10 +1118,10 @@ static struct nexthop *nexthop_create_group(struct net *net,
>
> /* spare group used for removals */
> nhg->spare = nexthop_grp_alloc(num_nh);
> - if (!nhg) {
> + if (!nhg->spare) {
> kfree(nhg);
> kfree(nh);
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> }
> nhg->spare->spare = nhg;
>
>
As Colin's similar patch[1] was rejected recently, this one also fixes the issue.
This is targeted at -net.
Fixes: 90f33bffa382 ("nexthops: don't modify published nexthop groups")
Acked-by: Nikolay Aleksandrov <[email protected]>
Thanks!
[1] https://lkml.org/lkml/2020/5/28/909
From: [email protected]
Date: Mon, 1 Jun 2020 13:12:01 +0200
> From: Patrick Eigensatz <[email protected]>
>
> After allocating the spare nexthop group it should be tested for kzalloc()
> returning NULL, instead the already used nexthop group (which cannot be
> NULL at this point) had been tested so far.
>
> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL.
>
> Coverity-id: 1463885
> Reported-by: Coverity <[email protected]>
> Signed-off-by: Patrick Eigensatz <[email protected]>
Applied, thank you.
On 01/06/2020 21:06, David Miller wrote:
> From: [email protected]
> Date: Mon, 1 Jun 2020 13:12:01 +0200
>
>> From: Patrick Eigensatz <[email protected]>
>>
>> After allocating the spare nexthop group it should be tested for kzalloc()
>> returning NULL, instead the already used nexthop group (which cannot be
>> NULL at this point) had been tested so far.
>>
>> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL.
>>
>> Coverity-id: 1463885
>> Reported-by: Coverity <[email protected]>
>> Signed-off-by: Patrick Eigensatz <[email protected]>
>
> Applied, thank you.
>
Hi Dave,
I see this patch in -net-next but it should've been in -net as I wrote in my
review[1]. This patch should go along with the recent nexthop set that fixes
a few bugs, since it could result in a null ptr deref if the spare group cannot
be allocated.
How would you like to proceed? Should it be submitted for -net as well?
Thanks,
Nik
[1] https://lkml.org/lkml/2020/6/1/391
On 02/06/2020 10:23, Nikolay Aleksandrov wrote:
> On 01/06/2020 21:06, David Miller wrote:
>> From: [email protected]
>> Date: Mon, 1 Jun 2020 13:12:01 +0200
>>
>>> From: Patrick Eigensatz <[email protected]>
>>>
>>> After allocating the spare nexthop group it should be tested for kzalloc()
>>> returning NULL, instead the already used nexthop group (which cannot be
>>> NULL at this point) had been tested so far.
>>>
>>> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL.
>>>
>>> Coverity-id: 1463885
>>> Reported-by: Coverity <[email protected]>
>>> Signed-off-by: Patrick Eigensatz <[email protected]>
>>
>> Applied, thank you.
>>
>
> Hi Dave,
> I see this patch in -net-next but it should've been in -net as I wrote in my
> review[1]. This patch should go along with the recent nexthop set that fixes
> a few bugs, since it could result in a null ptr deref if the spare group cannot
> be allocated.
Obviously I forgot to mention in my review that it should go to -stable with the
nexthop fix set.
> How would you like to proceed? Should it be submitted for -net as well?
>
> Thanks,
> Nik
>
> [1] https://lkml.org/lkml/2020/6/1/391
>
From: Nikolay Aleksandrov <[email protected]>
Date: Tue, 2 Jun 2020 10:23:09 +0300
> On 01/06/2020 21:06, David Miller wrote:
>> From: [email protected]
>> Date: Mon, 1 Jun 2020 13:12:01 +0200
>>
>>> From: Patrick Eigensatz <[email protected]>
>>>
>>> After allocating the spare nexthop group it should be tested for kzalloc()
>>> returning NULL, instead the already used nexthop group (which cannot be
>>> NULL at this point) had been tested so far.
>>>
>>> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL.
>>>
>>> Coverity-id: 1463885
>>> Reported-by: Coverity <[email protected]>
>>> Signed-off-by: Patrick Eigensatz <[email protected]>
>>
>> Applied, thank you.
>>
>
> Hi Dave,
> I see this patch in -net-next but it should've been in -net as I wrote in my
> review[1]. This patch should go along with the recent nexthop set that fixes
> a few bugs, since it could result in a null ptr deref if the spare group cannot
> be allocated.
> How would you like to proceed? Should it be submitted for -net as well?
When I'm leading up to the merge window I just toss everything into net-next
and still queue things to -stable as needed.
On 03/06/2020 00:01, David Miller wrote:
> From: Nikolay Aleksandrov <[email protected]>
> Date: Tue, 2 Jun 2020 10:23:09 +0300
>
>> On 01/06/2020 21:06, David Miller wrote:
>>> From: [email protected]
>>> Date: Mon, 1 Jun 2020 13:12:01 +0200
>>>
>>>> From: Patrick Eigensatz <[email protected]>
>>>>
>>>> After allocating the spare nexthop group it should be tested for kzalloc()
>>>> returning NULL, instead the already used nexthop group (which cannot be
>>>> NULL at this point) had been tested so far.
>>>>
>>>> Additionally, if kzalloc() fails, return ERR_PTR(-ENOMEM) instead of NULL.
>>>>
>>>> Coverity-id: 1463885
>>>> Reported-by: Coverity <[email protected]>
>>>> Signed-off-by: Patrick Eigensatz <[email protected]>
>>>
>>> Applied, thank you.
>>>
>>
>> Hi Dave,
>> I see this patch in -net-next but it should've been in -net as I wrote in my
>> review[1]. This patch should go along with the recent nexthop set that fixes
>> a few bugs, since it could result in a null ptr deref if the spare group cannot
>> be allocated.
>> How would you like to proceed? Should it be submitted for -net as well?
>
> When I'm leading up to the merge window I just toss everything into net-next
> and still queue things to -stable as needed.
>
Got it, in that case could you please queue the patch for -stable?
I checked https://patchwork.ozlabs.org/bundle/davem/stable/?state=* and
https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git
but didn't find it.
Thanks,
Nik