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

Yiqi Sun
2026-04-03T06:47:33Z

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@gmail.com>
> > ---
> > 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")