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

Message-ID: <20200713130929.0fc74fa1@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Mon, 13 Jul 2020 13:09:29 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Florian Fainelli <f.fainelli@...il.com>
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 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 :)

> 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.

That's just my 2¢..