[PATCH net v3] net: atm: Fix incorrect net_device lec check

[PATCH net v3] net: atm: Fix incorrect net_device lec check

[Lin Ma]

There are two sites in atm mpoa code that believe the fetched object
net_device is of lec type. However, both of them do just name checking
to ensure that the device name starts with "lec" pattern string.

That is, malicious user can hijack this by creating another device
starting with that pattern, thereby causing type confusion. For example,
create a *team* interface with lecX name, bind that interface and send
messages will get a crash like below:

[   18.450000] kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
[   18.452366] BUG: unable to handle page fault for address: ffff888005702a70
[   18.454253] #PF: supervisor instruction fetch in kernel mode
[   18.455058] #PF: error_code(0x0011) - permissions violation
[   18.455366] PGD 3801067 P4D 3801067 PUD 3802067 PMD 80000000056000e3
[   18.455725] Oops: 0011 [#1] PREEMPT SMP PTI
[   18.455966] CPU: 0 PID: 130 Comm: trigger Not tainted 6.1.90 #7
[   18.456921] RIP: 0010:0xffff888005702a70
[   18.457151] Code: .....
[   18.458168] RSP: 0018:ffffc90000677bf8 EFLAGS: 00010286
[   18.458461] RAX: ffff888005702a70 RBX: ffff888005702000 RCX: 000000000000001b
[   18.458850] RDX: ffffc90000677c10 RSI: ffff88800565e0a8 RDI: ffff888005702000
[   18.459248] RBP: ffffc90000677c68 R08: 0000000000000000 R09: 0000000000000000
[   18.459644] R10: 0000000000000000 R11: ffff888005702a70 R12: ffff88800556c000
[   18.460033] R13: ffff888005964900 R14: ffff8880054b4000 R15: ffff8880054b5000
[   18.460425] FS:  0000785e61b5a740(0000) GS:ffff88807dc00000(0000) knlGS:0000000000000000
[   18.460872] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   18.461183] CR2: ffff888005702a70 CR3: 00000000054c2000 CR4: 00000000000006f0
[   18.461580] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   18.461974] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   18.462368] Call Trace:
[   18.462518]  <TASK>
[   18.462645]  ? __die_body+0x64/0xb0
[   18.462856]  ? page_fault_oops+0x353/0x3e0
[   18.463092]  ? exc_page_fault+0xaf/0xd0
[   18.463322]  ? asm_exc_page_fault+0x22/0x30
[   18.463589]  ? msg_from_mpoad+0x431/0x9d0
[   18.463820]  ? vcc_sendmsg+0x165/0x3b0
[   18.464031]  vcc_sendmsg+0x20a/0x3b0
[   18.464238]  ? wake_bit_function+0x80/0x80
[   18.464511]  __sys_sendto+0x38c/0x3a0
[   18.464729]  ? percpu_counter_add_batch+0x87/0xb0
[   18.465002]  __x64_sys_sendto+0x22/0x30
[   18.465219]  do_syscall_64+0x6c/0xa0
[   18.465465]  ? preempt_count_add+0x54/0xb0
[   18.465697]  ? up_read+0x37/0x80
[   18.465883]  ? do_user_addr_fault+0x25e/0x5b0
[   18.466126]  ? exit_to_user_mode_prepare+0x12/0xb0
[   18.466435]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
[   18.466727] RIP: 0033:0x785e61be4407
[   18.467948] RSP: 002b:00007ffe61ae2150 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
[   18.468368] RAX: ffffffffffffffda RBX: 0000785e61b5a740 RCX: 0000785e61be4407
[   18.468758] RDX: 000000000000019c RSI: 00007ffe61ae21c0 RDI: 0000000000000003
[   18.469149] RBP: 00007ffe61ae2370 R08: 0000000000000000 R09: 0000000000000000
[   18.469542] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
[   18.469936] R13: 00007ffe61ae2498 R14: 0000785e61d74000 R15: 000057bddcbabd98

Refer to other net_device related subsystem, checking netlink_ops seems
like the correct way out, e.g., see how xgbe_netdev_event() validates
the netdev object. Hence, add correct comparison with lec_netdev_ops to
safeguard the casting. Since the *lec_netdev_ops* is defined in lec.c,
add one helpful function *is_netdev_lec()*.

By the way, this bug dates back to pre-git history (2.3.15), hence use
the first reference for tracking.

Signed-off-by: Lin Ma <linma@zju.edu.cn>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
---
V2 -> V3: add a helpful function to ensure the changes can compile.

 net/atm/lec.c | 5 +++++
 net/atm/lec.h | 2 ++
 net/atm/mpc.c | 5 ++++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index 73078306504c..46f0b5a00200 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -565,6 +565,11 @@ static const struct net_device_ops lec_netdev_ops = {
 	.ndo_set_rx_mode	= lec_set_multicast_list,
 };
 
+bool is_netdev_lec(const struct net_device *dev)
+{
+	return dev->netdev_ops == &lec_netdev_ops;
+}
+
 static const unsigned char lec_ctrl_magic[] = {
 	0xff,
 	0x00,
diff --git a/net/atm/lec.h b/net/atm/lec.h
index be0e2667bd8c..399726484097 100644
--- a/net/atm/lec.h
+++ b/net/atm/lec.h
@@ -152,4 +152,6 @@ struct lec_vcc_priv {
 
 #define LEC_VCC_PRIV(vcc)	((struct lec_vcc_priv *)((vcc)->user_back))
 
+bool is_netdev_lec(const struct net_device *dev);
+
 #endif				/* _LEC_H_ */
diff --git a/net/atm/mpc.c b/net/atm/mpc.c
index 583c27131b7d..48ce67a1cf4d 100644
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -275,6 +275,9 @@ static struct net_device *find_lec_by_itfnum(int itf)
 	sprintf(name, "lec%d", itf);
 	dev = dev_get_by_name(&init_net, name);
 
+	if (!dev || !is_netdev_lec(dev))
+		return NULL;
+
 	return dev;
 }
 
@@ -1006,7 +1009,7 @@ static int mpoa_event_listener(struct notifier_block *mpoa_notifier,
 	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
 
-	if (strncmp(dev->name, "lec", 3))
+	if (!is_netdev_lec(dev))
 		return NOTIFY_DONE; /* we are only interested in lec:s */
 
 	switch (event) {
-- 
2.17.1

Re: [PATCH net v3] net: atm: Fix incorrect net_device lec check

[Simon Horman]

On Thu, Jul 03, 2025 at 01:24:27PM +0800, Lin Ma wrote:
> There are two sites in atm mpoa code that believe the fetched object
> net_device is of lec type. However, both of them do just name checking
> to ensure that the device name starts with "lec" pattern string.
> 
> That is, malicious user can hijack this by creating another device
> starting with that pattern, thereby causing type confusion. For example,
> create a *team* interface with lecX name, bind that interface and send
> messages will get a crash like below:
> 
> [   18.450000] kernel tried to execute NX-protected page - exploit attempt? (uid: 0)
> [   18.452366] BUG: unable to handle page fault for address: ffff888005702a70
> [   18.454253] #PF: supervisor instruction fetch in kernel mode
> [   18.455058] #PF: error_code(0x0011) - permissions violation
> [   18.455366] PGD 3801067 P4D 3801067 PUD 3802067 PMD 80000000056000e3
> [   18.455725] Oops: 0011 [#1] PREEMPT SMP PTI
> [   18.455966] CPU: 0 PID: 130 Comm: trigger Not tainted 6.1.90 #7
> [   18.456921] RIP: 0010:0xffff888005702a70
> [   18.457151] Code: .....
> [   18.458168] RSP: 0018:ffffc90000677bf8 EFLAGS: 00010286
> [   18.458461] RAX: ffff888005702a70 RBX: ffff888005702000 RCX: 000000000000001b
> [   18.458850] RDX: ffffc90000677c10 RSI: ffff88800565e0a8 RDI: ffff888005702000
> [   18.459248] RBP: ffffc90000677c68 R08: 0000000000000000 R09: 0000000000000000
> [   18.459644] R10: 0000000000000000 R11: ffff888005702a70 R12: ffff88800556c000
> [   18.460033] R13: ffff888005964900 R14: ffff8880054b4000 R15: ffff8880054b5000
> [   18.460425] FS:  0000785e61b5a740(0000) GS:ffff88807dc00000(0000) knlGS:0000000000000000
> [   18.460872] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   18.461183] CR2: ffff888005702a70 CR3: 00000000054c2000 CR4: 00000000000006f0
> [   18.461580] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [   18.461974] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [   18.462368] Call Trace:
> [   18.462518]  <TASK>
> [   18.462645]  ? __die_body+0x64/0xb0
> [   18.462856]  ? page_fault_oops+0x353/0x3e0
> [   18.463092]  ? exc_page_fault+0xaf/0xd0
> [   18.463322]  ? asm_exc_page_fault+0x22/0x30
> [   18.463589]  ? msg_from_mpoad+0x431/0x9d0
> [   18.463820]  ? vcc_sendmsg+0x165/0x3b0
> [   18.464031]  vcc_sendmsg+0x20a/0x3b0
> [   18.464238]  ? wake_bit_function+0x80/0x80
> [   18.464511]  __sys_sendto+0x38c/0x3a0
> [   18.464729]  ? percpu_counter_add_batch+0x87/0xb0
> [   18.465002]  __x64_sys_sendto+0x22/0x30
> [   18.465219]  do_syscall_64+0x6c/0xa0
> [   18.465465]  ? preempt_count_add+0x54/0xb0
> [   18.465697]  ? up_read+0x37/0x80
> [   18.465883]  ? do_user_addr_fault+0x25e/0x5b0
> [   18.466126]  ? exit_to_user_mode_prepare+0x12/0xb0
> [   18.466435]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
> [   18.466727] RIP: 0033:0x785e61be4407
> [   18.467948] RSP: 002b:00007ffe61ae2150 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
> [   18.468368] RAX: ffffffffffffffda RBX: 0000785e61b5a740 RCX: 0000785e61be4407
> [   18.468758] RDX: 000000000000019c RSI: 00007ffe61ae21c0 RDI: 0000000000000003
> [   18.469149] RBP: 00007ffe61ae2370 R08: 0000000000000000 R09: 0000000000000000
> [   18.469542] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
> [   18.469936] R13: 00007ffe61ae2498 R14: 0000785e61d74000 R15: 000057bddcbabd98
> 
> Refer to other net_device related subsystem, checking netlink_ops seems
> like the correct way out, e.g., see how xgbe_netdev_event() validates
> the netdev object. Hence, add correct comparison with lec_netdev_ops to
> safeguard the casting. Since the *lec_netdev_ops* is defined in lec.c,
> add one helpful function *is_netdev_lec()*.
> 
> By the way, this bug dates back to pre-git history (2.3.15), hence use
> the first reference for tracking.
> 
> Signed-off-by: Lin Ma <linma@zju.edu.cn>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> ---
> V2 -> V3: add a helpful function to ensure the changes can compile.
> 
>  net/atm/lec.c | 5 +++++
>  net/atm/lec.h | 2 ++
>  net/atm/mpc.c | 5 ++++-
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/net/atm/lec.c b/net/atm/lec.c
> index 73078306504c..46f0b5a00200 100644
> --- a/net/atm/lec.c
> +++ b/net/atm/lec.c
> @@ -565,6 +565,11 @@ static const struct net_device_ops lec_netdev_ops = {
>  	.ndo_set_rx_mode	= lec_set_multicast_list,
>  };
>  
> +bool is_netdev_lec(const struct net_device *dev)
> +{
> +	return dev->netdev_ops == &lec_netdev_ops;
> +}
> +

Can start_mpc() run on dev before this function is called?
If so, does the check above still work as expected?

>  static const unsigned char lec_ctrl_magic[] = {
>  	0xff,
>  	0x00,
> diff --git a/net/atm/lec.h b/net/atm/lec.h
> index be0e2667bd8c..399726484097 100644
> --- a/net/atm/lec.h
> +++ b/net/atm/lec.h
> @@ -152,4 +152,6 @@ struct lec_vcc_priv {
>  
>  #define LEC_VCC_PRIV(vcc)	((struct lec_vcc_priv *)((vcc)->user_back))
>  
> +bool is_netdev_lec(const struct net_device *dev);
> +
>  #endif				/* _LEC_H_ */
> diff --git a/net/atm/mpc.c b/net/atm/mpc.c
> index 583c27131b7d..48ce67a1cf4d 100644
> --- a/net/atm/mpc.c
> +++ b/net/atm/mpc.c
> @@ -275,6 +275,9 @@ static struct net_device *find_lec_by_itfnum(int itf)
>  	sprintf(name, "lec%d", itf);
>  	dev = dev_get_by_name(&init_net, name);
>  
> +	if (!dev || !is_netdev_lec(dev))
> +		return NULL;
> +
>  	return dev;
>  }
>  

1) Is this code path reachable by non-lec devices?
2) Can the name of a lec device be changed?
   If so, does it cause a problem here?

> @@ -1006,7 +1009,7 @@ static int mpoa_event_listener(struct notifier_block *mpoa_notifier,
>  	if (!net_eq(dev_net(dev), &init_net))
>  		return NOTIFY_DONE;
>  
> -	if (strncmp(dev->name, "lec", 3))
> +	if (!is_netdev_lec(dev))
>  		return NOTIFY_DONE; /* we are only interested in lec:s */
>  
>  	switch (event) {
> -- 
> 2.17.1
> 

Re: [PATCH net v3] net: atm: Fix incorrect net_device lec check

[kernel test robot]

Hi Lin,

kernel test robot noticed the following build errors:

[auto build test ERROR on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Lin-Ma/net-atm-Fix-incorrect-net_device-lec-check/20250703-132727
base:   net/main
patch link:    https://lore.kernel.org/r/20250703052427.12626-1-linma%40zju.edu.cn
patch subject: [PATCH net v3] net: atm: Fix incorrect net_device lec check
config: x86_64-randconfig-078-20250705 (https://download.01.org/0day-ci/archive/20250705/202507050831.2GTrUnFN-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507050831.2GTrUnFN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507050831.2GTrUnFN-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "is_netdev_lec" [net/atm/mpoa.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net v3] net: atm: Fix incorrect net_device lec check

[Lin Ma]

Hi robot,

> 
> Hi Lin,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on net/main]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Lin-Ma/net-atm-Fix-incorrect-net_device-lec-check/20250703-132727
> base:   net/main
> patch link:    https://lore.kernel.org/r/20250703052427.12626-1-linma%40zju.edu.cn
> patch subject: [PATCH net v3] net: atm: Fix incorrect net_device lec check
> config: x86_64-randconfig-078-20250705 (https://download.01.org/0day-ci/archive/20250705/202507050831.2GTrUnFN-lkp@intel.com/config)
> compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507050831.2GTrUnFN-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202507050831.2GTrUnFN-lkp@intel.com/
> 
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
> >> ERROR: modpost: "is_netdev_lec" [net/atm/mpoa.ko] undefined!
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

You are right, this patch just compile when CONFIG_ATM=y but raise
error if CONFIG_ATM=m. I'm so stupid that I made this symbol mistake :(

Under fixing it currently.

Thanks
Lin

Re: [PATCH net v3] net: atm: Fix incorrect net_device lec check

[Lin Ma]

Hi Simon,

> Can start_mpc() run on dev before this function is called?
> If so, does the check above still work as expected?

Great point. I found two locatinos that call start_mpc(). One
is atm_mpoa_mpoad_attach() and the other is mpoa_event_listener().

Since this patch covers these two functions, I believe start_mpc()
run after the check.

However, since start_mpc() indeed replaces the netdev_ops. It seems
quite error-prone to perform type checking using that field. Moreover,
this patch raises a linking error as shown in
https://lore.kernel.org/oe-kbuild-all/202507050831.2GTrUnFN-lkp@intel.com/.
Hence, I will prepare version 4 and try to think about other solutions.

>
> 1) Is this code path reachable by non-lec devices?
>

Yes, though the mpoa_event_listener() is registered for the net/mpc module,
the notifier_block calls every registered listener.

/** ....
 *  Calls each function in a notifier chain in turn.  The functions
 *  run in an undefined context.
 *  All locking must be provided by the caller.
 *  ...
 */
int raw_notifier_call_chain(struct raw_notifier_head *nh,
        unsigned long val, void *v)

In another word, every registered listener is reachable and is responsible
for determining whether the event is relevant to them. And that is why we
should add a type check here.

> 2) Can the name of a lec device be changed?
   If so, does it cause a problem here?

To my knowledge, the changing of a net_device name is handled in
net/core/dev.c, which is a general functionality that an individual driver
cannot intervene.

int __dev_change_net_namespace(struct net_device *dev, struct net *net,
                   const char *pat, int new_ifindex)
{
    ...
        if (new_name[0]) /* Rename the netdev to prepared name */
        strscpy(dev->name, new_name, IFNAMSIZ);
    ...
}

Nice catch.
If a user changes a lec device name to something else that is not
prefixed with "lec", it causes functionality issues, as events
like NETDEV_UP/DOWN cannot reach the inner logic.

That will be another reason to adopt a fix.

Regards
Lin

Re: [PATCH net v3] net: atm: Fix incorrect net_device lec check

[Simon Horman]

On Sat, Jul 05, 2025 at 11:04:02AM +0800, Lin Ma wrote:
> Hi Simon,
> 
> > Can start_mpc() run on dev before this function is called?
> > If so, does the check above still work as expected?
> 
> Great point. I found two locatinos that call start_mpc(). One
> is atm_mpoa_mpoad_attach() and the other is mpoa_event_listener().
> 
> Since this patch covers these two functions, I believe start_mpc()
> run after the check.
> 
> However, since start_mpc() indeed replaces the netdev_ops. It seems
> quite error-prone to perform type checking using that field. Moreover,
> this patch raises a linking error as shown in
> https://lore.kernel.org/oe-kbuild-all/202507050831.2GTrUnFN-lkp@intel.com/.
> Hence, I will prepare version 4 and try to think about other solutions.

Thanks. I was thinking along the same lines.

> 
> >
> > 1) Is this code path reachable by non-lec devices?
> >
> 
> Yes, though the mpoa_event_listener() is registered for the net/mpc module,
> the notifier_block calls every registered listener.
> 
> /** ....
>  *  Calls each function in a notifier chain in turn.  The functions
>  *  run in an undefined context.
>  *  All locking must be provided by the caller.
>  *  ...
>  */
> int raw_notifier_call_chain(struct raw_notifier_head *nh,
>         unsigned long val, void *v)
> 
> In another word, every registered listener is reachable and is responsible
> for determining whether the event is relevant to them. And that is why we
> should add a type check here.

Understood, makes sense.

> 
> > 2) Can the name of a lec device be changed?
>    If so, does it cause a problem here?
> 
> To my knowledge, the changing of a net_device name is handled in
> net/core/dev.c, which is a general functionality that an individual driver
> cannot intervene.
> 
> int __dev_change_net_namespace(struct net_device *dev, struct net *net,
>                    const char *pat, int new_ifindex)
> {
>     ...
>         if (new_name[0]) /* Rename the netdev to prepared name */
>         strscpy(dev->name, new_name, IFNAMSIZ);
>     ...
> }
> 
> Nice catch.
> If a user changes a lec device name to something else that is not
> prefixed with "lec", it causes functionality issues, as events
> like NETDEV_UP/DOWN cannot reach the inner logic.
> 
> That will be another reason to adopt a fix.

Yes, that seems likely.
But perhaps such a fix is orthogonal from the one that started this thread.