From: Colin Ian King <[email protected]>
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 <[email protected]>
---
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
On 28/05/2020 17:51, Colin King wrote:
> From: Colin Ian King <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
On 5/28/20 8:53 AM, Nikolay Aleksandrov wrote:
> On 28/05/2020 17:51, Colin King wrote:
>> From: Colin Ian King <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
I missed that as well.
Reviewed-by: David Ahern <[email protected]>
Patch needs to go to -net
On 28/05/2020 17:53, Nikolay Aleksandrov wrote:
> On 28/05/2020 17:51, Colin King wrote:
>> From: Colin Ian King <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
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 <[email protected]>
Date: Tue May 26 12:56:15 2020 -0600
nexthops: don't modify published nexthop groups
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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>
>
> 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 <[email protected]>
> 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
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 <[email protected]>
>>>>>
>>>>> 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 <[email protected]>
>>>>> ---
>>>>> 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 <[email protected]>
>>>>
>>>
>>> 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 <[email protected]>
>>> 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
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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> ---
>>>> 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 <[email protected]>
>>>
>>
>> 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 <[email protected]>
>> 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.
From: Colin King <[email protected]>
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?