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

[Tobias Waldekranz <tobias@waldekranz.com>]

On fre, mar 21, 2025 at 11:10, Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:
> Hi Tobias,
>
> On Fri, 21 Mar 2025 10:03:23 +0100
> Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
>> Protect the parser TCAM/SRAM memory, and the cached (shadow) SRAM
>> information, from concurrent modifications.
>> 
>> Both the TCAM and SRAM tables are indirectly accessed by configuring
>> an index register that selects the row to read or write to. This means
>> that operations must be atomic in order to, e.g., avoid spreading
>> writes across multiple rows. Since the shadow SRAM array is used to
>> find free rows in the hardware table, it must also be protected in
>> order to avoid TOCTOU errors where multiple cores allocate the same
>> row.
>> 
>> This issue was detected in a situation where `mvpp2_set_rx_mode()` ran
>> concurrently on two CPUs. In this particular case the
>> MVPP2_PE_MAC_UC_PROMISCUOUS entry was corrupted, causing the
>> classifier unit to drop all incoming unicast - indicated by the
>> `rx_classifier_drops` counter.
>> 
>> Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
>> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>
> I gave it a quick test with simple tcam-based vlan filtering and uc/mc
> filtering, it looks and behaves fine but I probably didn't stress it
> enough to hit the races you encountered. Still, the features that used
> to work still work :)

Good to hear! :)

I have tried to stress it by concurrently hammering on the promisc
setting on multiple ports, while adding/removing MDB entries without any
issues.

I've also ran the original reproducer about 10-20x the number of
iterations it usually took to trigger the issue.

> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
>
> Thanks a lot,

Thanks for reviewing and testing!