Re: Re: [PATCH] net/mpls: fix missing NULL check in mpls_valid_fib_dump_req

[Yiqi Sun]

On 3/26/26 11:25 AM, Paolo Abeni wrote:
> On 3/23/26 8:15 AM, sunichi wrote:
> > The attribute tb[RTA_OIF] is dereferenced without verifying if it is NULL.
> > If this attribute is missing in the user netlink message, it will cause a
> > NULL pointer dereference and kernel panic.
> > 
> > Add the necessary check before using the pointer to prevent the crash.
> > 
> > Signed-off-by: sunichi <sunyiqixm@xxxxxxxxx>
> > ---
> >  net/mpls/af_mpls.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> > index d5417688f69e..28bbea30aae3 100644
> > --- a/net/mpls/af_mpls.c
> > +++ b/net/mpls/af_mpls.c
> > @@ -2174,6 +2174,8 @@ static int mpls_valid_fib_dump_req(struct net *net, const struct nlmsghdr *nlh,
> >  		int ifindex;
> >  
> >  		if (i == RTA_OIF) {
> > +			if (!tb[i])
> > +				return -EINVAL;
> >  			ifindex = nla_get_u32(tb[i]);
> >  			filter->dev = dev_get_by_index_rcu(net, ifindex);
> >  			if (!filter->dev)
> 
> If you reorder the check I think it will lead to better code:
> 
> ---
> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
> index b32311f5cbf7..41510dce5329 100644
> --- a/net/mpls/af_mpls.c
> +++ b/net/mpls/af_mpls.c
> @@ -2170,13 +2170,16 @@ static int mpls_valid_fib_dump_req(struct net
> *net, const struct nlmsghdr *nlh,
>         for (i = 0; i <= RTA_MAX; ++i) {
>                 int ifindex;
> 
> +               if (!tb[i])
> +                       continue;
> +
>                 if (i == RTA_OIF) {
>                         ifindex = nla_get_u32(tb[i]);
>                         filter->dev = dev_get_by_index_rcu(net, ifindex);
>                         if (!filter->dev)
>                                 return -ENODEV;
>                         filter->filter_set = 1;
> -               } else if (tb[i]) {
> +               } else {
>                         NL_SET_ERR_MSG_MOD(extack, "Unsupported
> attribute in dump request");
>                         return -EINVAL;
>                 }

Thanks for the suggestion! The reordered version looks better to me.

And sorry, forgot to add:
Fixes: 196cfebf8972 ("net/mpls: Handle kernel side filtering of route dumps")