netdev - Re: [net-next,v22,2/7] net: mtip: The L2 switch driver for imx287

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: <20260204102452.180eb416@wsk>
Date: Wed, 4 Feb 2026 10:24:52 +0100
From: Łukasz Majewski <lukasz.majewski@...lbox.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: andrew@...n.ch, shawnguo@...nel.org, krzk+dt@...nel.org,
 linux-kernel@...r.kernel.org, edumazet@...gle.com, netdev@...r.kernel.org,
 pabeni@...hat.com, andrew+netdev@...n.ch, davem@...emloft.net,
 conor+dt@...nel.org, horms@...nel.org, richardcochran@...il.com,
 robh@...nel.org, imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 devicetree@...r.kernel.org, wahrenst@....net, s.hauer@...gutronix.de,
 kernel@...gutronix.de, festevam@...il.com
Subject: Re: [net-next,v22,2/7] net: mtip: The L2 switch driver for imx287

Hi Jakub,

> On Tue, 3 Feb 2026 21:19:55 +0100 Łukasz Majewski wrote:
> > > > +static void mtip_write_atable(struct switch_enet_private *fep,
> > > > int index,
> > > > +			      u32 write_lo, u32 write_hi)
> > > > +{
> > > > +	struct addr_table64b_entry __iomem *atable_base =
> > > > +		fep->hwentry->mtip_table64b_entry;
> > > > +
> > > > +	writel(write_lo, &atable_base[index].lo);
> > > > +	writel(write_hi, &atable_base[index].hi);
> > > > +}      
> > > 
> > > Can these functions race with concurrent access? Looking at the
> > > callers, mtip_write_atable is called from two different paths:
> > > 
> > > 1. Static entry updates: mtip_config_switch ->
> > > esw_mac_addr_static -> mtip_update_atable_static ->
> > > mtip_write_atable (no lock held)
> > > 
> > > 2. Dynamic entry updates: timer callback -> mtip_mgnt_timer ->
> > >    mtip_atable_dynamicms_learn_migration ->
> > > mtip_update_atable_dynamic1 -> mtip_write_atable (learn_lock held)
> > > 
> > > The learn_lock only protects the dynamic entry path. The static
> > > entry path runs during link changes (mtip_switch_restart called
> > > from mtip_adjust_link) without lock protection.
> > > 
> > > Both paths can access the same hash block in the address table
> > > (determined by GET_BLOCK_PTR(hash)). If the timer fires during a
> > > link change callback, both can concurrently access the table,
> > > potentially causing torn reads (reading .lo from one entry
> > > version and .hi from another) or torn writes (the entry is in an
> > > inconsistent state between the two writel calls).
> > > 
> > > Would extending learn_lock to protect all address table access
> > > work, or is a separate hw_lock needed for hardware register
> > > access? 
> > 
> > This is handled in another way:
> > 
> > 1. Partial write is not possible as this IP block handles it in
> > order (with some kind of 'latch' registers):
> > 
> > "VFxxx Controller Reference Manual, Rev. 0, 10/2016"
> > 11.5.4 MAC address lookup table
> > 
> > "Each entry must be written or read with the low address accessed
> > first followed by the high address"
> > 
> > 2. The code for dynamic IP writing will not "touch" the entries for
> > "static" MAC addresses - Figure 11-70 - bit 49 is "Record Type":
> > 	1 - static entry
> > 	0 - dynamic entry
> > 
> > IMHO, we are "safe" here.  
> 
> My reading of the AI's comment was that there is no lock in SW so two
> SW threads can write:
> 
>    Thread A   Thread B
>    write_lo
>               write_lo
>               write_hi
>    write_hi

Ok. Then I can add "atable_access" spin lock to prevent from this
situation.

-- 
Best regards,

Łukasz Majewski