netdev - Re: [PATCH net] net: lan78xx: Disable hardware vlan filtering in promiscuous mode

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: <20201119093109.1b5c9cd8@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Thu, 19 Nov 2020 09:31:09 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Markus Blöchl <Markus.Bloechl@...tronik.com>
Cc:     Vladimir Oltean <olteanv@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Ido Schimmel <idosch@...sch.org>, Andrew Lunn <andrew@...n.ch>,
        Alexander Duyck <alexander.duyck@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        "David S. Miller" <davem@...emloft.net>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH net] net: lan78xx: Disable hardware vlan filtering in
 promiscuous mode

On Thu, 19 Nov 2020 16:37:51 +0100 Markus Blöchl wrote:
> Implementation
> ==============
> 
> I then tried to come up with a solution in net/core that would
> universally disable vlan filtering in promiscuous mode.

Thanks for taking a look!

> Removing the features in `netdev_fix_features` is easily done, but
> updating the active set of features whenever IFF_PROMISC changes
> seems hard.
> 
> `__dev_set_promiscuity` is often called in atomic context but
> `.ndo_set_features` can sleep in many drivers.

Are there paths other than __dev_set_rx_mode() which would call
__dev_set_promiscuity() in atomic context? The saving grace about 
__dev_set_rx_mode() is that it sets promisc explicitly to disable 
unicast filtering (dev->uc_promisc), so IMO that case
(dev->promiscuity == dev->uc_promisc) does not need to disable VLAN
filtering.

But IDK if that's the only atomic path.

> Adding a work_queue or similar to every net_device seems clumsy and
> inappropriate.
> 
> Rewriting ndo_set_features of all drivers to be atomic is not a task
> you should ask from me...
> 
> Calling `netdev_update_features` after every entrypoint that locks
> the rtnl seems too error-prone and also clumsy.
> 
> Only updating the features when promiscuity was explicitly requested
> by userspace in `dev_change_flags` might leave the device in a
> weird inconsistent state.
> 
> Continue to let each driver enforce the kernels definition of
> promiscuity. They know how to update the features atomically.
> Then I am back at my original patch...
> 
> I'm afraid, I might need some guidance on how to approach this.