netdev - Re: Circular dependency between DSA switch driver and tagging protocol driver

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: <e0567cfe-d8b6-ed92-02c6-e45dd108d7d7@gmail.com>
Date:   Wed, 8 Sep 2021 15:14:51 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: Circular dependency between DSA switch driver and tagging
 protocol driver



On 9/8/2021 3:08 PM, Vladimir Oltean wrote:
> Hi,
> 
> Since commits 566b18c8b752 ("net: dsa: sja1105: implement TX
> timestamping for SJA1110") and 994d2cbb08ca ("net: dsa: tag_sja1105: be
> dsa_loop-safe"), net/dsa/tag_sja1105.ko has gained a build and insmod
> time dependency on drivers/net/dsa/sja1105.ko, due to several symbols
> exported by the latter and used by the former.
> 
> So first one needs to insmod sja1105.ko, then insmod tag_sja1105.ko.
> 
> But dsa_port_parse_cpu returns -EPROBE_DEFER when dsa_tag_protocol_get
> returns -ENOPROTOOPT. It means, there is no DSA_TAG_PROTO_SJA1105 in the
> list of tagging protocols known by DSA, try again later. There is a
> runtime dependency for DSA to have the tagging protocol loaded. Combined
> with the symbol dependency, this is a de facto circular dependency.
> 
> So when we first insmod sja1105.ko, nothing happens, probing is deferred.
> 
> Then when we insmod tag_sja1105.ko, we expect the DSA probing to kick
> off where it left from, and probe the switch too.
> 
> However this does not happen because the deferred probing list in the
> device core is reconsidered for a new attempt only if a driver is bound
> to a new device. But DSA tagging protocols are drivers with no struct
> device.
> 
> One can of course manually kick the driver after the two insmods:
> 
> echo spi0.1 > /sys/bus/spi/drivers/sja1105/bind
> 
> and this works, but automatic module loading based on modaliases will be
> broken if both tag_sja1105.ko and sja1105.ko are modules, and sja1105 is
> the last device to get a driver bound to it.
> 
> Where is the problem?

I'd say with 994d2cbb08ca, since the tagger now requires visibility into 
sja1105_switch_ops which is not great, to say the least. You could solve 
this by:

- splitting up the sja1150 between a library that contains 
sja1105_switch_ops and does not contain the driver registration code

- finding a different way to do a dsa_switch_ops pointer comparison, by 
e.g.: maintaining a boolean in dsa_port that tracks whether a particular 
driver is backing that port
-- 
Florian