1. Patchew
  2. linux
  3. View series

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

sunichi posted 1 patch 1 week, 4 days ago 
net/mpls/af_mpls.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] net/mpls: fix missing NULL check in mpls_valid_fib_dump_req
Posted by sunichi 1 week, 4 days ago 
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)
-- 
2.34.1
Re: [PATCH] net/mpls: fix missing NULL check in mpls_valid_fib_dump_req
Posted by Paolo Abeni 1 week, 1 day ago 
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;
                }
Re: Re: [PATCH] net/mpls: fix missing NULL check in mpls_valid_fib_dump_req
Posted by Yiqi Sun 14 hours ago 
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")
Re: [PATCH] net/mpls: fix missing NULL check in mpls_valid_fib_dump_req
Posted by Nikolay Aleksandrov 1 week, 4 days ago 
On Mon, Mar 23, 2026 at 03:15:15PM +0800, 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)
> -- 
> 2.34.1
> 

Why necessary ? Did you actually test and see any problem?

RTA_OIF is parsed as NLA_U32 according to rtm_mpls_policy and
nla_for_each_attr walks over all attributes in the msg which
means it is set and we must have at least that many bytes
available for the attribute. So how can it be NULL?
Re: [PATCH] net/mpls: fix missing NULL check in mpls_valid_fib_dump_req
Posted by sunichi 1 week, 4 days ago 
On Mon, 23 Mar 2026 at 16:52, Nikolay Aleksandrov <razor@blackwall.org>
wrote:
> On Mon, Mar 23, 2026 at 03:15:15PM +0800, 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)
> > --
> > 2.34.1
> >
>
> Why necessary ? Did you actually test and see any problem?

Yes, I've triggered null-ptr-derefer.

```
root@syzkaller:~# ./mpls-dos
[   12.591512] BUG: kernel NULL pointer dereference, address:
0000000000000004
[   12.591817] #PF: supervisor read access in kernel mode
[   12.591924] #PF: error_code(0x0000) - not-present page
[   12.592052] PGD 1041c9067 P4D 1041c9067 PUD 102273067 PMD 0
[   12.592371] Oops: Oops: 0000 [#1] SMP NOPTI
[   12.592860] CPU: 1 UID: 0 PID: 167 Comm: mpls-dos Not tainted 7.0.0-rc4
#12 PREEMPT(lazy)
[   12.593051] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.15.0-1 04/01/2014
[   12.593305] RIP: 0010:mpls_valid_fib_dump_req+0xdf/0x1f0
[   12.593710] Code: ......
[   12.594032] RSP: 0018:ffffc9000035b908 EFLAGS: 00000246
[   12.594149] RAX: 0000000000000000 RBX: 0000000000000004 RCX:
0000000000000000
[   12.594277] RDX: ffffc9000035bab8 RSI: 0000000000000000 RDI:
ffffffff834bfd80
[   12.594393] RBP: 0000000000000005 R08: 0000000000000003 R09:
0000000000000000
[   12.594507] R10: ffffc9000035b908 R11: ffffc9000035b908 R12:
ffffffff834bfd80
[   12.594623] R13: ffffc9000035bab8 R14: ffffc9000035ba48 R15:
ffff888102114330
[   12.594778] FS:  000000002f71d3c0(0000) GS:ffff8881b88fc000(0000)
knlGS:0000000000000000
[   12.594916] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   12.595014] CR2: 0000000000000004 CR3: 00000001020b2000 CR4:
00000000000006f0
[   12.595283] Call Trace:
[   12.596008]  <TASK>
[   12.596308]  mpls_dump_routes+0x167/0x1e0
[   12.596471]  netlink_dump+0x156/0x450
```

> RTA_OIF is parsed as NLA_U32 according to rtm_mpls_policy and
> nla_for_each_attr walks over all attributes in the msg which
> means it is set and we must have at least that many bytes
> available for the attribute. So how can it be NULL?

Yes, NULL can pass the check, so you can see tb[i]-null-check
everywhere in the kernel.

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.