netdev - Re: question lan9303: DSA concurrency and locking,

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 15 Nov 2017 15:08:07 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Egil Hjelmeland <privat@...l-hjelmeland.no>
Cc:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev <netdev@...r.kernel.org>
Subject: Re: question lan9303: DSA concurrency and locking,

On Wed, Nov 15, 2017 at 01:08:22PM +0100, Egil Hjelmeland wrote:
> Hi experts
> 
> I am hoping for some guidance.
> 
> Does DSA offer any protection against concurrent calls of dsa_switch_ops?

Hi Egil

DSA itself does not.

There are various upper locks, which protect some calls, in some ways.
e.g. phy ops are protected by the mdio lock. stats calls are protected
by the rtnl lock, as well as some other calls. And other locks protect
other things.

But nothing gives you protection across them all.

For the mv88e6xxx driver, we took the simple approach. We generally
take a lock at the beginning of each of the dsa_swtich_ops functions,
and release it at the end. Since all accesses to the chip go through
two read/write functions, we also have code in them to detect when
they are called without holding the lock.

Some driver writers worry about performance in some situations, and
want finer grain locking. So they have multiple locks. When reviewing
drivers i will look for obvious locking issues, but don't look too
deeply. Without knowing the chip, it has hard for me to know if
something is safe or not. So i would not be surprised if there are
locking issues in some drivers.

> The most "interesting" part of the lan9303 driver that has no locking is the
> ALR (=fdb/mdb). ALR access is a sequence of register operations. Anyway it
> is very unlikely that mdb related calls are reentered. But if it can happen,
> it would mean that IGMP snooping can go wrong. (Which is actually very bad
> in our applications.)
> 
> Is this something to worry about?

I would suggest looking a bit higher in the stack. fdb/mdb operations
come via switchdev, and have a notification mechanism between slave.c
and port.c. Check if that notification mechanism enforces
serialisation. Also, check that everything actually does go though
this notification mechanism. Maybe the dump operations do not?

And then check the lower levels of the driver. If say statistics
operations are performed at the same time as fdb/mdb, can the register
accesses get interleaved? If they can, is that actually a problem for
the hardware?

	 Andrew