All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: Xu Yang <xu.yang_2@nxp.com>, Andrew Lunn <andrew@lunn.ch>,
	hkallweit1@gmail.com, pabeni@redhat.com, netdev@vger.kernel.org,
	imx@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [RESEND] net: phy: fix NULL pointer dereference in phy_polling_mode()
Date: Thu, 7 Aug 2025 14:45:04 +0200	[thread overview]
Message-ID: <aJSf0JaBl4cKphFi@pengutronix.de> (raw)
In-Reply-To: <aJSSNg4aZNfoqqZh@shell.armlinux.org.uk>

On Thu, Aug 07, 2025 at 12:47:02PM +0100, Russell King (Oracle) wrote:
> On Thu, Aug 07, 2025 at 07:21:46PM +0800, Xu Yang wrote:
> > Hi Russell and Andrew,
> > 
> > With more debug on why asix_devices.c driver is creating so many mdio devices,
> > I found the mdio->phy_mask setting may be missing.
> 
> mdio->phy_mask is really only a workaround/optimisation to prevent
> the automatic scanning of the MDIO bus.
> 
> If we know for certain that we're only interested in a PHY at a
> certain set of addresses, then it's appropriate to tell the MDIO/phylib
> layer not to bother scanning the other addresses, but this will mean
> if the driver uses e.g. phy_find_first(), it will find the first PHY
> amongst those that phy_mask allows to be scanned, rather than the first
> on the bus.
> 
> In other words... it's dependent on the driver.
> 
> > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> > index 9b0318fb50b5..9fba1cb17134 100644
> > --- a/drivers/net/usb/asix_devices.c
> > +++ b/drivers/net/usb/asix_devices.c
> > @@ -676,6 +676,7 @@ static int ax88772_init_mdio(struct usbnet *dev)
> >         priv->mdio->read = &asix_mdio_bus_read;
> >         priv->mdio->write = &asix_mdio_bus_write;
> >         priv->mdio->name = "Asix MDIO Bus";
> > +       priv->mdio->phy_mask = ~BIT(priv->phy_addr);
> >         /* mii bus name is usb-<usb bus number>-<usb device number> */
> >         snprintf(priv->mdio->id, MII_BUS_ID_SIZE, "usb-%03d:%03d",
> >                  dev->udev->bus->busnum, dev->udev->devnum);
> > 
> > Is this the right thing to do?
> 
> If we're only expecting a MDIO device at priv->phy_addr, then I
> guess it's fine. Looking at the driver, I don't understand the
> mixture of dev->mii.* and priv->mdio->*, and sadly I don't have
> time to look in depth at this driver to work that out.

Hm, I guess, with this change there will be a subtile regression.
In case of an external PHYs the ax88772_init_phy() is using PHYlib to
suspend the internal PHY.

May be:
  priv->mdio->phy_mask = ~(BIT(priv->phy_addr) | BIT(AX_EMBD_PHY_ADDR));

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2025-08-07 12:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-06  8:29 [RESEND] net: phy: fix NULL pointer dereference in phy_polling_mode() Xu Yang
2025-08-06  8:45 ` Russell King (Oracle)
2025-08-06  8:56   ` Xu Yang
2025-08-06 13:01     ` Russell King (Oracle)
2025-08-06 14:14       ` Xu Yang
2025-08-06 15:01         ` Andrew Lunn
2025-08-06 16:47           ` Russell King (Oracle)
2025-08-07  9:23             ` Xu Yang
2025-08-07 11:21               ` Xu Yang
2025-08-07 11:47                 ` Russell King (Oracle)
2025-08-07 12:45                   ` Oleksij Rempel [this message]
2025-08-07 12:58                     ` Andrew Lunn
2025-08-07 14:02                       ` Oleksij Rempel
2025-08-08 10:26                     ` Xu Yang
2025-08-08 10:17                   ` Xu Yang
2025-08-07 12:55                 ` Andrew Lunn
2025-08-07  9:10           ` Xu Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJSf0JaBl4cKphFi@pengutronix.de \
    --to=o.rempel@pengutronix.de \
    --cc=andrew@lunn.ch \
    --cc=hkallweit1@gmail.com \
    --cc=imx@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=xu.yang_2@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.