Re: [PATCH net-next v2 3/4] nexthop: Fix out-of-bounds access during attribute validation

[Ido Schimmel <idosch@nvidia.com>]

On Mon, Mar 11, 2024 at 09:28:30PM -0600, David Ahern wrote:
> On 3/11/24 10:23 AM, Ido Schimmel wrote:
> Reviewed-by: David Ahern <dsahern@kernel.org>

Thanks!

> 
> 
> > diff --git a/tools/testing/selftests/net/fib_nexthops.sh b/tools/testing/selftests/net/fib_nexthops.sh
> > index d5a281aadbac..ac0b2c6a5761 100755
> > --- a/tools/testing/selftests/net/fib_nexthops.sh
> > +++ b/tools/testing/selftests/net/fib_nexthops.sh
> > @@ -2066,6 +2066,12 @@ basic()
> >  	run_cmd "$IP nexthop get id 1"
> >  	log_test $? 2 "Nexthop get on non-existent id"
> >  
> > +	run_cmd "$IP nexthop del id 1"
> > +	log_test $? 2 "Nexthop del with non-existent id"
> > +
> > +	run_cmd "$IP nexthop del id 1 group 1/2/3/4/5/6/7/8"
> > +	log_test $? 2 "Nexthop del with non-existent id and extra attributes"
> > +
> >  	# attempt to create nh without a device or gw - fails
> >  	run_cmd "$IP nexthop add id 1"
> >  	log_test $? 2 "Nexthop with no device or gateway"
> 
> The basic() group of tests do not have a delete, so this is a good
> addition. However, the ipv6_fcnal and ipv4_fcnal do have a del - seems
> like those tests should have caught the out of bounds access.

There are deletion tests, but they only provide the nexthop ID and the
purpose of providing some bogus attribute ("group" in this case) was to
trigger the out-of-bounds access in validate_nla():

pt = &policy[type];

As rtm_nh_policy_del does not contain an entry for NHA_GROUP.