netdev - Re: [PATCH net-next 0/3] net: Preserve netdev

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: <22f861f0-39f0-8c26-6571-d10903f53297@gmail.com>
Date:   Mon, 13 Jul 2020 14:20:01 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, andrew@...n.ch, vivien.didelot@...il.com,
        mkubecek@...e.cz, davem@...emloft.net
Subject: Re: [PATCH net-next 0/3] net: Preserve netdev_ops equality tests



On 7/13/2020 1:09 PM, Jakub Kicinski wrote:
> On Sun, 12 Jul 2020 15:16:22 -0700 Florian Fainelli wrote:
>> Hi David, Jakub,
>>
>> This patch series addresses a long standing with no known impact today
>> with the overloading of netdev_ops done by the DSA layer.
> 
> Do you plan to make use of this comparison? Or trying to protect the
> MAC driver from misbehaving because it's unaware the DSA may replace
> its ops? For non-DSA experts I think it may be worth stating :)

We have at least one network device driver that is always used in a DSA
set-up (bcmsysport.c) which does check DSA notifiers against its own
netdev_ops, however this happens before the mangling of DSA netdev_ops
is done, so the check is not defeated there.

> 
>> First we introduce a ndo_equal netdev_ops function pointer, then we have
>> DSA utilize it, and finally all in tree users are converted to using
>> either netdev_ops_equal() or __netdev_ops_equal() (for const struct
>> net_device reference).
> 
> The experience with TCP ULPs made me dislike hijacking ops :( 
> Maybe it's just my limited capability to comprehend complex systems
> but the moment there is more than one entity that tries to insert
> itself as a proxy, ordering etc. gets quite hairy.. Perhaps we 
> have some well understood rules for ndo replacement but if that's not
> the case I prefer the interception to be done explicitly in the caller.
> (e.g. add separate dsa_ops to struct net_device and call that prior to/
> /instead of calling the ndo).
> 
> At the very least I'd think it's better to create an explicit hierarchy
> of the ops by linking them (add "const struct net_device_ops *base_ops"
> to ndos) rather than one-off callback for comparisons.

Initially I was going to introduce a way to do recursive operations, but
the problem with that approach is that you need to impose an ordering
within the core about how operations are invoked.

The idea to add dsa_ops as a singleton that DSA would provide (very much
like the recent ethtool_phy_ops introduction) is probably the sanest
approach.

Thanks for the suggestions and review Jakub!
-- 
Florian