netdev - Re: [PATCH v3 net-next 6/9] net: dsa: introduce locking for the address lists on CPU and DSA ports

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

Message-ID: <d953d624-a2ba-0052-34cc-c1f9ec8730c2@gmail.com>
Date:   Fri, 22 Oct 2021 10:37:10 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Vladimir Oltean <olteanv@...il.com>,
        UNGLinuxDriver@...rochip.com, DENG Qingfang <dqfext@...il.com>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        George McCollister <george.mccollister@...il.com>,
        John Crispin <john@...ozen.org>,
        Aleksander Jan Bajkowski <olek2@...pl>,
        Egil Hjelmeland <privat@...l-hjelmeland.no>,
        Oleksij Rempel <o.rempel@...gutronix.de>,
        Prasanna Vengateshan <prasanna.vengateshan@...rochip.com>,
        Ansuel Smith <ansuelsmth@...il.com>,
        Alvin Šipraga <alsi@...g-olufsen.dk>
Subject: Re: [PATCH v3 net-next 6/9] net: dsa: introduce locking for the
 address lists on CPU and DSA ports

On 10/22/21 10:27 AM, Vladimir Oltean wrote:
> Now that the rtnl_mutex is going away for dsa_port_{host_,}fdb_{add,del},
> no one is serializing access to the address lists that DSA keeps for the
> purpose of reference counting on shared ports (CPU and cascade ports).
> 
> It can happen for one dsa_switch_do_fdb_del to do list_del on a dp->fdbs
> element while another dsa_switch_do_fdb_{add,del} is traversing dp->fdbs.
> We need to avoid that.
> 
> Currently dp->mdbs is not at risk, because dsa_switch_do_mdb_{add,del}
> still runs under the rtnl_mutex. But it would be nice if it would not
> depend on that being the case. So let's introduce a mutex per port (the
> address lists are per port too) and share it between dp->mdbs and
> dp->fdbs.
> 
> The place where we put the locking is interesting. It could be tempting
> to put a DSA-level lock which still serializes calls to
> .port_fdb_{add,del}, but it would still not avoid concurrency with other
> driver code paths that are currently under rtnl_mutex (.port_fdb_dump,
> .port_fast_age). So it would add a very false sense of security (and
> adding a global switch-wide lock in DSA to resynchronize with the
> rtnl_lock is also counterproductive and hard).
> 
> So the locking is intentionally done only where the dp->fdbs and dp->mdbs
> lists are traversed. That means, from a driver perspective, that
> .port_fdb_add will be called with the dp->addr_lists_lock mutex held on
> the CPU port, but not held on user ports. This is done so that driver
> writers are not encouraged to rely on any guarantee offered by
> dp->addr_lists_lock.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian