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] [day] [month] [year] [list]

Message-ID: <20250320182738.67108ea4@fedora.home>
Date: Thu, 20 Mar 2025 18:27:38 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Andrew Lunn <andrew@...n.ch>
Cc: Tobias Waldekranz <tobias@...dekranz.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 Thu, 20 Mar 2025 14:14:16 +0100
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...
> 
> Maybe the design was that RTNL is supposed to protect this, but things
> are happening outside of it? It would of helped if the code had put in
> some ASSERT_RTNL() calls to both indicate this was the idea, and to
> find cases where it was not actually true.

I think this was definitely missed. I added some of it back then, and I
certainly didn't consider non-rtnl protected paths. an ASSERT_RTNL
would've been a good idea indeed :(

With netdev_lock closing in, I think Tobias's approach is better. We
can't rely on a netdev_lock to protect the parser as it's shared
accross multiple netdevs.

Maxime