netdev - [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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Nov 2020 16:39:58 +0100
From:   Markus Blöchl <markus.bloechl@...tronik.com>
To:     Woojung Huh <woojung.huh@...rochip.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        netdev@...r.kernel.org
Subject: [PATCH net] net: lan78xx: Disable hardware vlan filtering in
 promiscuous mode

The rx-vlan-filter feature flag prevents unexpected tagged frames on
the wire from reaching the kernel in promiscuous mode.
Disable this offloading feature in the lan7800 controller whenever
IFF_PROMISC is set and make sure that the hardware features
are updated when IFF_PROMISC changes.

Signed-off-by: Markus Blöchl <markus.bloechl@...tronik.com>
---

Notes:
    When sniffing ethernet using a LAN7800 ethernet controller, vlan-tagged
    frames are silently dropped by the controller due to the
    RFE_CTL_VLAN_FILTER flag being set by default since commit
    4a27327b156e("net: lan78xx: Add support for VLAN filtering.").

    In order to receive those tagged frames it is necessary to manually disable
    rx vlan filtering using ethtool ( `ethtool -K ethX rx-vlan-filter off` or
    corresponding ioctls ). Setting all bits in the vlan filter table to 1 is
    an even worse approach, imho.

    As a user I would probably expect that setting IFF_PROMISC should be enough
    in all cases to receive all valid traffic.
    Therefore I think this behaviour is a bug in the driver, since other
    drivers (notably ixgbe) automatically disable rx-vlan-filter when
    IFF_PROMISC is set. Please correct me if I am wrong here.

 drivers/net/usb/lan78xx.c | 33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 65b315bc60ab..ac6c0beeac21 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2324,13 +2324,15 @@ static int lan78xx_set_mac_addr(struct net_device *netdev, void *p)
 }

 /* Enable or disable Rx checksum offload engine */
-static int lan78xx_set_features(struct net_device *netdev,
-                               netdev_features_t features)
+static void lan78xx_update_features(struct net_device *netdev,
+                                   netdev_features_t features)
 {
        struct lan78xx_net *dev = netdev_priv(netdev);
        struct lan78xx_priv *pdata = (struct lan78xx_priv *)(dev->data[0]);
        unsigned long flags;
-       int ret;
+
+       if (netdev->flags & IFF_PROMISC)
+               features &= ~NETIF_F_HW_VLAN_CTAG_FILTER;

        spin_lock_irqsave(&pdata->rfe_ctl_lock, flags);

@@ -2353,12 +2355,30 @@ static int lan78xx_set_features(struct net_device *netdev,
                pdata->rfe_ctl &= ~RFE_CTL_VLAN_FILTER_;

        spin_unlock_irqrestore(&pdata->rfe_ctl_lock, flags);
+}
+
+static int lan78xx_set_features(struct net_device *netdev,
+                               netdev_features_t features)
+{
+       struct lan78xx_net *dev = netdev_priv(netdev);
+       struct lan78xx_priv *pdata = (struct lan78xx_priv *)(dev->data[0]);
+       int ret;
+
+       lan78xx_update_features(netdev, features);

        ret = lan78xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);

        return 0;
 }

+static void lan78xx_set_rx_mode(struct net_device *netdev)
+{
+       /* Enable or disable checksum offload engines */
+       lan78xx_update_features(netdev, netdev->features);
+
+       lan78xx_set_multicast(netdev);
+}
+
 static void lan78xx_deferred_vlan_write(struct work_struct *param)
 {
        struct lan78xx_priv *pdata =
@@ -2528,10 +2548,7 @@ static int lan78xx_reset(struct lan78xx_net *dev)
        pdata->rfe_ctl |= RFE_CTL_BCAST_EN_ | RFE_CTL_DA_PERFECT_;
        ret = lan78xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);

-       /* Enable or disable checksum offload engines */
-       lan78xx_set_features(dev->net, dev->net->features);
-
-       lan78xx_set_multicast(dev->net);
+       lan78xx_set_rx_mode(dev->net);

        /* reset PHY */
        ret = lan78xx_read_reg(dev, PMT_CTL, &buf);
@@ -3613,7 +3630,7 @@ static const struct net_device_ops lan78xx_netdev_ops = {
        .ndo_set_mac_address    = lan78xx_set_mac_addr,
        .ndo_validate_addr      = eth_validate_addr,
        .ndo_do_ioctl           = phy_do_ioctl_running,
-       .ndo_set_rx_mode        = lan78xx_set_multicast,
+       .ndo_set_rx_mode        = lan78xx_set_rx_mode,
        .ndo_set_features       = lan78xx_set_features,
        .ndo_vlan_rx_add_vid    = lan78xx_vlan_rx_add_vid,
        .ndo_vlan_rx_kill_vid   = lan78xx_vlan_rx_kill_vid,

base-commit: 4e0396c59559264442963b349ab71f66e471f84d
--
2.29.2


Impressum/Imprint: https://www.ipetronik.com/impressum