From: Tobias Waldekranz <tobias@waldekranz.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>,
davem@davemloft.net, kuba@kernel.org, marcin.s.wojtas@gmail.com,
linux@armlinux.org.uk, edumazet@google.com, pabeni@redhat.com,
ezequiel.garcia@free-electrons.com, netdev@vger.kernel.org
Subject: Re: [PATCH net] net: mvpp2: Prevent parser TCAM memory corruption
Date: Thu, 20 Mar 2025 14:38:20 +0100 [thread overview]
Message-ID: <87wmcjakkz.fsf@waldekranz.com> (raw)
In-Reply-To: <16143c70-de5a-4f30-ad29-eae33d2e5b0b@lunn.ch>
On tor, mar 20, 2025 at 14:14, Andrew Lunn <andrew@lunn.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.
next prev parent reply other threads:[~2025-03-20 13:38 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-20 9:17 [PATCH net] net: mvpp2: Prevent parser TCAM memory corruption Tobias Waldekranz
2025-03-20 9:57 ` Maxime Chevallier
2025-03-20 10:47 ` Tobias Waldekranz
2025-03-20 13:14 ` Andrew Lunn
2025-03-20 13:38 ` Tobias Waldekranz [this message]
2025-03-20 17:27 ` Maxime Chevallier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87wmcjakkz.fsf@waldekranz.com \
--to=tobias@waldekranz.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=ezequiel.garcia@free-electrons.com \
--cc=kuba@kernel.org \
--cc=linux@armlinux.org.uk \
--cc=marcin.s.wojtas@gmail.com \
--cc=maxime.chevallier@bootlin.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).