netdev - Re: [PATCH net] net: mvpp2: Prevent parser TCAM memory corruption

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87wmcjakkz.fsf@waldekranz.com>
Date: Thu, 20 Mar 2025 14:38:20 +0100
From: Tobias Waldekranz <tobias@...dekranz.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Maxime Chevallier <maxime.chevallier@...tlin.com>, davem@...emloft.net,
 kuba@...nel.org, marcin.s.wojtas@...il.com, linux@...linux.org.uk,
 edumazet@...gle.com, pabeni@...hat.com,
 ezequiel.garcia@...e-electrons.com, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: mvpp2: Prevent parser TCAM memory corruption

On tor, mar 20, 2025 at 14:14, Andrew Lunn <andrew@...n.ch> wrote:
>> We still need to disable bottom halves though, right?  Because otherwise
>> we could reach mvpp2_set_rx_mode() from net-rx by processing an IGMP/MLD
>> frame, for example.
>
> Ah, that answers the question i was asking myself. Why does RTNL not
> cover this...

If we zoom out from the single-CPU perspective, another path not covered
by RTNL (which is how I ran into this) is via deferred switchdev event
processing. Here are the stack traces which confirmed by suspicions:

[   32.161265] CPU: 2 PID: 6198 Comm: 55-init.ip Not tainted 6.6.52 #1
[   32.166445] Hardware name: fooboard
[   32.166450] Call trace:
[   32.166453]  dump_backtrace+0x98/0xf8
[   32.166464]  show_stack+0x20/0x38
[   32.166469]  dump_stack_lvl+0x48/0x60
[   32.166478]  dump_stack+0x18/0x28
[   32.166484]  mvpp2_prs_hw_write.isra.0+0x194/0x1b8 [mvpp2]
[   32.166508]  mvpp2_prs_mac_promisc_set+0xac/0x168 [mvpp2]
[   32.166526]  mvpp2_set_rx_mode+0x15c/0x190 [mvpp2]
[   32.166543]  __dev_set_rx_mode+0x68/0xb0
[   32.166549]  dev_uc_sync+0x80/0x98
[   32.166555]  vlan_dev_set_rx_mode+0x34/0x50
[   32.166562]  __dev_set_rx_mode+0x68/0xb0
[   32.166567]  dev_uc_add+0x94/0xc0
[   32.166571]  br_fdb_sync_static+0x58/0x120
[   32.166577]  br_add_if+0x720/0x7a8
[   32.166582]  br_add_slave+0x1c/0x30
[   32.166589]  do_set_master+0x98/0xc8
[   32.166596]  do_setlink+0x2a4/0xec0
[   32.166602]  __rtnl_newlink+0x51c/0x890
[   32.166607]  rtnl_newlink+0x58/0x90
[   32.166611]  rtnetlink_rcv_msg+0x12c/0x380
[   32.166616]  netlink_rcv_skb+0x60/0x138
[   32.166623]  rtnetlink_rcv+0x20/0x38
[   32.166630]  netlink_unicast+0x2fc/0x370
[   32.166636]  netlink_sendmsg+0x1ac/0x428
[   32.166642]  ____sys_sendmsg+0x1d8/0x2c8
[   32.166648]  ___sys_sendmsg+0xb4/0x110
[   32.166653]  __sys_sendmsg+0x8c/0xf0
[   32.166658]  __arm64_sys_sendmsg+0x2c/0x40
[   32.166663]  invoke_syscall+0x50/0x128
[   32.166670]  el0_svc_common.constprop.0+0x48/0xf0
[   32.166677]  do_el0_svc+0x24/0x38
[   32.166684]  el0_svc+0x40/0xe8
[   32.166692]  el0t_64_sync_handler+0x120/0x130
[   32.166698]  el0t_64_sync+0x190/0x198

[   32.166704] CPU: 3 PID: 11 Comm: kworker/u8:0 Not tainted 6.6.52 #1
[   32.166711] Hardware name: fooboard
[   32.166716] Workqueue: dsa_ordered dsa_slave_switchdev_event_work
[   32.170392] Call trace:
[   32.170395]  dump_backtrace+0x98/0xf8
[   32.170400]  show_stack+0x20/0x38
[   32.170405]  dump_stack_lvl+0x48/0x60
[   32.170411]  dump_stack+0x18/0x28
[   32.170417]  mvpp2_prs_hw_write.isra.0+0x74/0x1b8 [mvpp2]
[   32.170436]  mvpp2_prs_mac_promisc_set+0xac/0x168 [mvpp2]
[   32.170454]  mvpp2_set_rx_mode+0x15c/0x190 [mvpp2]
[   32.170471]  __dev_set_rx_mode+0x68/0xb0
[   32.170476]  dev_uc_add+0x94/0xc0
[   32.170481]  dsa_port_bridge_host_fdb_add+0x90/0x108
[   32.170487]  dsa_slave_switchdev_event_work+0x1a0/0x1b8
[   32.170495]  process_one_work+0x148/0x3b8
[   32.170500]  worker_thread+0x32c/0x450
[   32.170505]  kthread+0x118/0x128
[   32.170511]  ret_from_fork+0x10/0x20

These are captured from mvpp2_prs_hw_write(), patched with some
temporary atomic_{add,sub}_return()s to detect concurrent execution.

> Maybe the design was that RTNL is supposed to protect this, but things
> are happening outside of it?

Yeah I was thinking the same thing.