All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] net/ipv4/nexthop: check the return value of nexthop_find_by_id()
@ 2022-09-17  2:30 Li Zhong
  2022-09-17  8:29 ` Nikolay Aleksandrov
  0 siblings, 1 reply; 4+ messages in thread
From: Li Zhong @ 2022-09-17  2:30 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: pabeni, kuba, edumazet, davem, yoshfuji, dsahern, Li Zhong

Check the return value of nexthop_find_by_id(), which could be NULL on
when not found. So we check to avoid null pointer dereference.

Signed-off-by: Li Zhong <floridsleeves@gmail.com>
---
 net/ipv4/nexthop.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
index 853a75a8fbaf..9f91bb78eed5 100644
--- a/net/ipv4/nexthop.c
+++ b/net/ipv4/nexthop.c
@@ -2445,6 +2445,10 @@ static struct nexthop *nexthop_create_group(struct net *net,
 		struct nh_info *nhi;
 
 		nhe = nexthop_find_by_id(net, entry[i].id);
+		if (!nhe) {
+			err = -EINVAL;
+			goto out_no_nh;
+		}
 		if (!nexthop_get(nhe)) {
 			err = -ENOENT;
 			goto out_no_nh;
-- 
2.25.1


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

* Re: [PATCH v1] net/ipv4/nexthop: check the return value of nexthop_find_by_id()
  2022-09-17  2:30 [PATCH v1] net/ipv4/nexthop: check the return value of nexthop_find_by_id() Li Zhong
@ 2022-09-17  8:29 ` Nikolay Aleksandrov
  2022-09-17 14:46   ` David Ahern
  0 siblings, 1 reply; 4+ messages in thread
From: Nikolay Aleksandrov @ 2022-09-17  8:29 UTC (permalink / raw)
  To: Li Zhong, linux-kernel, netdev
  Cc: pabeni, kuba, edumazet, davem, yoshfuji, dsahern

On 17/09/2022 05:30, Li Zhong wrote:
> Check the return value of nexthop_find_by_id(), which could be NULL on
> when not found. So we check to avoid null pointer dereference.
> 
> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
> ---
>  net/ipv4/nexthop.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> index 853a75a8fbaf..9f91bb78eed5 100644
> --- a/net/ipv4/nexthop.c
> +++ b/net/ipv4/nexthop.c
> @@ -2445,6 +2445,10 @@ static struct nexthop *nexthop_create_group(struct net *net,
>  		struct nh_info *nhi;
>  
>  		nhe = nexthop_find_by_id(net, entry[i].id);
> +		if (!nhe) {
> +			err = -EINVAL;
> +			goto out_no_nh;
> +		}
>  		if (!nexthop_get(nhe)) {
>  			err = -ENOENT;
>  			goto out_no_nh;

These are validated in nh_check_attr_group() and should exist at this point.
Since remove_nexthop() should run under rtnl I don't see a way for a nexthop
to disappear after nh_check_attr_group() and before nexthop_create_group().

Did you notice a problem or have a stack dump?


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

* Re: [PATCH v1] net/ipv4/nexthop: check the return value of nexthop_find_by_id()
  2022-09-17  8:29 ` Nikolay Aleksandrov
@ 2022-09-17 14:46   ` David Ahern
  2022-09-18  1:54     ` Li Zhong
  0 siblings, 1 reply; 4+ messages in thread
From: David Ahern @ 2022-09-17 14:46 UTC (permalink / raw)
  To: Nikolay Aleksandrov, Li Zhong, linux-kernel, netdev
  Cc: pabeni, kuba, edumazet, davem, yoshfuji

On 9/17/22 2:29 AM, Nikolay Aleksandrov wrote:
> On 17/09/2022 05:30, Li Zhong wrote:
>> Check the return value of nexthop_find_by_id(), which could be NULL on
>> when not found. So we check to avoid null pointer dereference.
>>
>> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
>> ---
>>  net/ipv4/nexthop.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
>> index 853a75a8fbaf..9f91bb78eed5 100644
>> --- a/net/ipv4/nexthop.c
>> +++ b/net/ipv4/nexthop.c
>> @@ -2445,6 +2445,10 @@ static struct nexthop *nexthop_create_group(struct net *net,
>>  		struct nh_info *nhi;
>>  
>>  		nhe = nexthop_find_by_id(net, entry[i].id);
>> +		if (!nhe) {
>> +			err = -EINVAL;
>> +			goto out_no_nh;
>> +		}
>>  		if (!nexthop_get(nhe)) {
>>  			err = -ENOENT;
>>  			goto out_no_nh;
> 
> These are validated in nh_check_attr_group() and should exist at this point.
> Since remove_nexthop() should run under rtnl I don't see a way for a nexthop
> to disappear after nh_check_attr_group() and before nexthop_create_group().
> 

exactly. That lookup can't fail because the ids have been validated and
all of this is under rtnl preventing nexthop removes.


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

* Re: [PATCH v1] net/ipv4/nexthop: check the return value of nexthop_find_by_id()
  2022-09-17 14:46   ` David Ahern
@ 2022-09-18  1:54     ` Li Zhong
  0 siblings, 0 replies; 4+ messages in thread
From: Li Zhong @ 2022-09-18  1:54 UTC (permalink / raw)
  To: David Ahern
  Cc: Nikolay Aleksandrov, linux-kernel, netdev, pabeni, kuba,
	edumazet, davem, yoshfuji

On Sat, Sep 17, 2022 at 7:46 AM David Ahern <dsahern@kernel.org> wrote:
>
> On 9/17/22 2:29 AM, Nikolay Aleksandrov wrote:
> > On 17/09/2022 05:30, Li Zhong wrote:
> >> Check the return value of nexthop_find_by_id(), which could be NULL on
> >> when not found. So we check to avoid null pointer dereference.
> >>
> >> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
> >> ---
> >>  net/ipv4/nexthop.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c
> >> index 853a75a8fbaf..9f91bb78eed5 100644
> >> --- a/net/ipv4/nexthop.c
> >> +++ b/net/ipv4/nexthop.c
> >> @@ -2445,6 +2445,10 @@ static struct nexthop *nexthop_create_group(struct net *net,
> >>              struct nh_info *nhi;
> >>
> >>              nhe = nexthop_find_by_id(net, entry[i].id);
> >> +            if (!nhe) {
> >> +                    err = -EINVAL;
> >> +                    goto out_no_nh;
> >> +            }
> >>              if (!nexthop_get(nhe)) {
> >>                      err = -ENOENT;
> >>                      goto out_no_nh;
> >
> > These are validated in nh_check_attr_group() and should exist at this point.
> > Since remove_nexthop() should run under rtnl I don't see a way for a nexthop
> > to disappear after nh_check_attr_group() and before nexthop_create_group().
> >
>
> exactly. That lookup can't fail because the ids have been validated and
> all of this is under rtnl preventing nexthop removes.
>

Thanks for all your replies. That makes sense to me.

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

end of thread, other threads:[~2022-09-18  1:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-17  2:30 [PATCH v1] net/ipv4/nexthop: check the return value of nexthop_find_by_id() Li Zhong
2022-09-17  8:29 ` Nikolay Aleksandrov
2022-09-17 14:46   ` David Ahern
2022-09-18  1:54     ` Li Zhong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.