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

[Simon Horman <horms@kernel.org>]

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
>