kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
@ 2020-05-28 14:51 Colin King
  2020-05-28 14:53 ` Nikolay Aleksandrov
  2020-05-30  0:15 ` David Miller
  0 siblings, 2 replies; 8+ messages in thread
From: Colin King @ 2020-05-28 14:51 UTC (permalink / raw)
  To: David Ahern, David S . Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Jakub Kicinski, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

The allocation failure check for nhg->spare is currently checking
the pointer nhg rather than nhg->spare which is never false. Fix
this by checking nhg->spare instead.

Addresses-Coverity: ("Logically dead code")
Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/ipv4/nexthop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index ebafa5ed91ac..97423d6f2de9 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -1185,7 +1185,7 @@ 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;
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
  2020-05-28 14:51 [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare Colin King
@ 2020-05-28 14:53 ` Nikolay Aleksandrov
  2020-05-28 14:55   ` David Ahern
  2020-05-28 14:55   ` Nikolay Aleksandrov
  2020-05-30  0:15 ` David Miller
  1 sibling, 2 replies; 8+ messages in thread
From: Nikolay Aleksandrov @ 2020-05-28 14:53 UTC (permalink / raw)
  To: Colin King, David Ahern, David S . Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Jakub Kicinski, netdev
  Cc: kernel-janitors, linux-kernel

On 28/05/2020 17:51, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The allocation failure check for nhg->spare is currently checking
> the pointer nhg rather than nhg->spare which is never false. Fix
> this by checking nhg->spare instead.
> 
> Addresses-Coverity: ("Logically dead code")
> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  net/ipv4/nexthop.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index ebafa5ed91ac..97423d6f2de9 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
> @@ -1185,7 +1185,7 @@ 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;
> 

Good catch, embarrassing copy paste error :-/
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
  2020-05-28 14:53 ` Nikolay Aleksandrov
@ 2020-05-28 14:55   ` David Ahern
  2020-05-28 14:55   ` Nikolay Aleksandrov
  1 sibling, 0 replies; 8+ messages in thread
From: David Ahern @ 2020-05-28 14:55 UTC (permalink / raw)
  To: Nikolay Aleksandrov, Colin King, David Ahern, David S . Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev
  Cc: kernel-janitors, linux-kernel

On 5/28/20 8:53 AM, Nikolay Aleksandrov wrote:
> On 28/05/2020 17:51, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The allocation failure check for nhg->spare is currently checking
>> the pointer nhg rather than nhg->spare which is never false. Fix
>> this by checking nhg->spare instead.
>>
>> Addresses-Coverity: ("Logically dead code")
>> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  net/ipv4/nexthop.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>> index ebafa5ed91ac..97423d6f2de9 100644
>> --- a/net/ipv4/nexthop.c
>> +++ b/net/ipv4/nexthop.c
>> @@ -1185,7 +1185,7 @@ 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;
>>
> 
> Good catch, embarrassing copy paste error :-/
> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> 

I missed that as well.

Reviewed-by: David Ahern <dsahern@gmail.com>

Patch needs to go to -net

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
  2020-05-28 14:53 ` Nikolay Aleksandrov
  2020-05-28 14:55   ` David Ahern
@ 2020-05-28 14:55   ` Nikolay Aleksandrov
  2020-05-28 15:53     ` Colin Ian King
  1 sibling, 1 reply; 8+ messages in thread
From: Nikolay Aleksandrov @ 2020-05-28 14:55 UTC (permalink / raw)
  To: Colin King, David Ahern, David S . Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Jakub Kicinski, netdev
  Cc: kernel-janitors, linux-kernel

On 28/05/2020 17:53, Nikolay Aleksandrov wrote:
> On 28/05/2020 17:51, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The allocation failure check for nhg->spare is currently checking
>> the pointer nhg rather than nhg->spare which is never false. Fix
>> this by checking nhg->spare instead.
>>
>> Addresses-Coverity: ("Logically dead code")
>> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  net/ipv4/nexthop.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>> index ebafa5ed91ac..97423d6f2de9 100644
>> --- a/net/ipv4/nexthop.c
>> +++ b/net/ipv4/nexthop.c
>> @@ -1185,7 +1185,7 @@ 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;
>>
> 
> Good catch, embarrassing copy paste error :-/
> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> 

Wait - that should be targeted at -net, that's where the fixes went.
And the fixes tag is wrong, nhg->spare was very recently added by:
commit 90f33bffa382
Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date:   Tue May 26 12:56:15 2020 -0600

    nexthops: don't modify published nexthop groups

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
  2020-05-28 14:55   ` Nikolay Aleksandrov
@ 2020-05-28 15:53     ` Colin Ian King
  2020-05-28 15:55       ` Nikolay Aleksandrov
  0 siblings, 1 reply; 8+ messages in thread
From: Colin Ian King @ 2020-05-28 15:53 UTC (permalink / raw)
  To: Nikolay Aleksandrov, David Ahern, David S . Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev
  Cc: kernel-janitors, linux-kernel

On 28/05/2020 15:55, Nikolay Aleksandrov wrote:
> On 28/05/2020 17:53, Nikolay Aleksandrov wrote:
>> On 28/05/2020 17:51, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> The allocation failure check for nhg->spare is currently checking
>>> the pointer nhg rather than nhg->spare which is never false. Fix
>>> this by checking nhg->spare instead.
>>>
>>> Addresses-Coverity: ("Logically dead code")
>>> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>>  net/ipv4/nexthop.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>>> index ebafa5ed91ac..97423d6f2de9 100644
>>> --- a/net/ipv4/nexthop.c
>>> +++ b/net/ipv4/nexthop.c
>>> @@ -1185,7 +1185,7 @@ 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;
>>>
>>
>> Good catch, embarrassing copy paste error :-/
>> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>
> 
> Wait - that should be targeted at -net, that's where the fixes went.
> And the fixes tag is wrong, nhg->spare was very recently added by:
> commit 90f33bffa382
> Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date:   Tue May 26 12:56:15 2020 -0600
> 
>     nexthops: don't modify published nexthop groups
> 

Do you require me to fix this up and re-send?

Colin

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
  2020-05-28 15:53     ` Colin Ian King
@ 2020-05-28 15:55       ` Nikolay Aleksandrov
  2020-05-28 15:56         ` Colin Ian King
  0 siblings, 1 reply; 8+ messages in thread
From: Nikolay Aleksandrov @ 2020-05-28 15:55 UTC (permalink / raw)
  To: Colin Ian King, David Ahern, David S . Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Jakub Kicinski, netdev
  Cc: kernel-janitors, linux-kernel

On 28/05/2020 18:53, Colin Ian King wrote:
> On 28/05/2020 15:55, Nikolay Aleksandrov wrote:
>> On 28/05/2020 17:53, Nikolay Aleksandrov wrote:
>>> On 28/05/2020 17:51, Colin King wrote:
>>>> From: Colin Ian King <colin.king@canonical.com>
>>>>
>>>> The allocation failure check for nhg->spare is currently checking
>>>> the pointer nhg rather than nhg->spare which is never false. Fix
>>>> this by checking nhg->spare instead.
>>>>
>>>> Addresses-Coverity: ("Logically dead code")
>>>> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>> ---
>>>>  net/ipv4/nexthop.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>>>> index ebafa5ed91ac..97423d6f2de9 100644
>>>> --- a/net/ipv4/nexthop.c
>>>> +++ b/net/ipv4/nexthop.c
>>>> @@ -1185,7 +1185,7 @@ 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;
>>>>
>>>
>>> Good catch, embarrassing copy paste error :-/
>>> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>>
>>
>> Wait - that should be targeted at -net, that's where the fixes went.
>> And the fixes tag is wrong, nhg->spare was very recently added by:
>> commit 90f33bffa382
>> Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> Date:   Tue May 26 12:56:15 2020 -0600
>>
>>     nexthops: don't modify published nexthop groups
>>
> 
> Do you require me to fix this up and re-send?
> 
> Colin
> 

Up to you, it will go to the same stable releases as that fix used the same
Fixes tag, so it's really not an issue.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
  2020-05-28 15:55       ` Nikolay Aleksandrov
@ 2020-05-28 15:56         ` Colin Ian King
  0 siblings, 0 replies; 8+ messages in thread
From: Colin Ian King @ 2020-05-28 15:56 UTC (permalink / raw)
  To: Nikolay Aleksandrov, David Ahern, David S . Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, netdev
  Cc: kernel-janitors, linux-kernel

On 28/05/2020 16:55, Nikolay Aleksandrov wrote:
> On 28/05/2020 18:53, Colin Ian King wrote:
>> On 28/05/2020 15:55, Nikolay Aleksandrov wrote:
>>> On 28/05/2020 17:53, Nikolay Aleksandrov wrote:
>>>> On 28/05/2020 17:51, Colin King wrote:
>>>>> From: Colin Ian King <colin.king@canonical.com>
>>>>>
>>>>> The allocation failure check for nhg->spare is currently checking
>>>>> the pointer nhg rather than nhg->spare which is never false. Fix
>>>>> this by checking nhg->spare instead.
>>>>>
>>>>> Addresses-Coverity: ("Logically dead code")
>>>>> Fixes: 430a049190de ("nexthop: Add support for nexthop groups")
>>>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>>>> ---
>>>>>  net/ipv4/nexthop.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>>>>> index ebafa5ed91ac..97423d6f2de9 100644
>>>>> --- a/net/ipv4/nexthop.c
>>>>> +++ b/net/ipv4/nexthop.c
>>>>> @@ -1185,7 +1185,7 @@ 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;
>>>>>
>>>>
>>>> Good catch, embarrassing copy paste error :-/
>>>> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>>>
>>>
>>> Wait - that should be targeted at -net, that's where the fixes went.
>>> And the fixes tag is wrong, nhg->spare was very recently added by:
>>> commit 90f33bffa382
>>> Author: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>>> Date:   Tue May 26 12:56:15 2020 -0600
>>>
>>>     nexthops: don't modify published nexthop groups
>>>
>>
>> Do you require me to fix this up and re-send?
>>
>> Colin
>>
> 
> Up to you, it will go to the same stable releases as that fix used the same
> Fixes tag, so it's really not an issue.
> 
OK, I'll let it slip and won't send a V2. Thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare
  2020-05-28 14:51 [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare Colin King
  2020-05-28 14:53 ` Nikolay Aleksandrov
@ 2020-05-30  0:15 ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2020-05-30  0:15 UTC (permalink / raw)
  To: colin.king
  Cc: dsahern, kuznet, yoshfuji, kuba, netdev, kernel-janitors, linux-kernel

From: Colin King <colin.king@canonical.com>
Date: Thu, 28 May 2020 15:51:14 +0100

> @@ -1185,7 +1185,7 @@ static struct nexthop *nexthop_create_group(struct net *net,
>  
>  	/* spare group used for removals */
>  	nhg->spare = nexthop_grp_alloc(num_nh);

I don't even see this line in the current net-next tree nor any references
to ->spare.

What am I missing?

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-05-30  0:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28 14:51 [PATCH][net-next] nexthop: fix incorrect allocation failure on nhg->spare Colin King
2020-05-28 14:53 ` Nikolay Aleksandrov
2020-05-28 14:55   ` David Ahern
2020-05-28 14:55   ` Nikolay Aleksandrov
2020-05-28 15:53     ` Colin Ian King
2020-05-28 15:55       ` Nikolay Aleksandrov
2020-05-28 15:56         ` Colin Ian King
2020-05-30  0:15 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).