netdev - [bug report] net-forcedeth: Add internal loopback support for forcedeth NICs.

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

Message-ID: <Y7g1Skvt3d0YDHmf@kili>
Date:   Fri, 6 Jan 2023 17:50:50 +0300
From:   Dan Carpenter <error27@...il.com>
To:     netdev@...r.kernel.org
Cc:     Sanjay Hortikar <horti@...gle.com>
Subject: [bug report] net-forcedeth: Add internal loopback support for
 forcedeth NICs.

[ This bug is 11 years old now but it's pretty obviously buggy so maybe
  someone wants to take a look.  I have no idea why this warning is
  only showing up now.  - dan ]

The patch e19df76a1113: "net-forcedeth: Add internal loopback support
for forcedeth NICs." from Nov 11, 2011, leads to the following Smatch
static checker warning:

drivers/net/ethernet/nvidia/forcedeth.c:4889 nv_set_loopback() warn: sleeping in atomic context
drivers/net/ethernet/nvidia/forcedeth.c:4915 nv_set_loopback() warn: sleeping in atomic context

drivers/net/ethernet/nvidia/forcedeth.c
    4874 static int nv_set_loopback(struct net_device *dev, netdev_features_t features)
    4875 {
    4876         struct fe_priv *np = netdev_priv(dev);
    4877         unsigned long flags;
    4878         u32 miicontrol;
    4879         int err, retval = 0;
    4880 
    4881         spin_lock_irqsave(&np->lock, flags);
                 ^^^^^^^^^^^^^^^^^
Holding a spin lock.

    4882         miicontrol = mii_rw(dev, np->phyaddr, MII_BMCR, MII_READ);
    4883         if (features & NETIF_F_LOOPBACK) {
    4884                 if (miicontrol & BMCR_LOOPBACK) {
    4885                         spin_unlock_irqrestore(&np->lock, flags);
    4886                         netdev_info(dev, "Loopback already enabled\n");
    4887                         return 0;
    4888                 }
--> 4889                 nv_disable_irq(dev);

You can't call disable_irq() with preempt disabled.

    4890                 /* Turn on loopback mode */
    4891                 miicontrol |= BMCR_LOOPBACK | BMCR_FULLDPLX | BMCR_SPEED1000;
    4892                 err = mii_rw(dev, np->phyaddr, MII_BMCR, miicontrol);
    4893                 if (err) {
    4894                         retval = PHY_ERROR;
    4895                         spin_unlock_irqrestore(&np->lock, flags);
    4896                         phy_init(dev);
    4897                 } else {
    4898                         if (netif_running(dev)) {
    4899                                 /* Force 1000 Mbps full-duplex */
    4900                                 nv_force_linkspeed(dev, NVREG_LINKSPEED_1000,
    4901                                                                          1);
    4902                                 /* Force link up */
    4903                                 netif_carrier_on(dev);
    4904                         }
    4905                         spin_unlock_irqrestore(&np->lock, flags);
    4906                         netdev_info(dev,
    4907                                 "Internal PHY loopback mode enabled.\n");
    4908                 }
    4909         } else {
    4910                 if (!(miicontrol & BMCR_LOOPBACK)) {
    4911                         spin_unlock_irqrestore(&np->lock, flags);
    4912                         netdev_info(dev, "Loopback already disabled\n");
    4913                         return 0;
    4914                 }
    4915                 nv_disable_irq(dev);

Same.

    4916                 /* Turn off loopback */
    4917                 spin_unlock_irqrestore(&np->lock, flags);
    4918                 netdev_info(dev, "Internal PHY loopback mode disabled.\n");
    4919                 phy_init(dev);
    4920         }
    4921         msleep(500);
    4922         spin_lock_irqsave(&np->lock, flags);
    4923         nv_enable_irq(dev);
    4924         spin_unlock_irqrestore(&np->lock, flags);
    4925 
    4926         return retval;
    4927 }

regards,
dan carpenter