From: Lukas Funke <[email protected]>
When there is no driver associated with the phydev, there will be a
nullptr access. The commit checks if the phydev driver is set before
access.
Signed-off-by: Lukas Funke <[email protected]>
---
drivers/net/phy/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1135e63a4a76..4de83e3cab77 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -136,7 +136,7 @@ EXPORT_SYMBOL(phy_print_status);
static int phy_config_interrupt(struct phy_device *phydev, bool interrupts)
{
phydev->interrupts = interrupts ? 1 : 0;
- if (phydev->drv->config_intr)
+ if (phydev->drv && phydev->drv->config_intr)
return phydev->drv->config_intr(phydev);
return 0;
--
2.30.2
On Mon, Nov 20, 2023 at 10:32:54AM +0100, Lukas Funke wrote:
> From: Lukas Funke <[email protected]>
>
> When there is no driver associated with the phydev, there will be a
> nullptr access. The commit checks if the phydev driver is set before
> access.
What's the call path that we encounter a NULL drv pointer?
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russel,
On 20.11.2023 10:51, Russell King (Oracle) wrote:
> On Mon, Nov 20, 2023 at 10:32:54AM +0100, Lukas Funke wrote:
>> From: Lukas Funke <[email protected]>
>>
>> When there is no driver associated with the phydev, there will be a
>> nullptr access. The commit checks if the phydev driver is set before
>> access.
>
> What's the call path that we encounter a NULL drv pointer?
The patch is a bit older and the path is reconstructed from my memory:
macb_phylink_connect -> phylink_of_phy_connect -> of_phy_connect ->
phy_connect_direct -> phy_request_interrupt
It happend when we used the Xilinx gmii2rgmii phy driver. We did a
missconfiguration in the dt and bumped into the nullpointer exception.
Since other functions like phy_aneg_done() also check for driver
existence I thought it would be a good addition.
>
> Thanks.
>
On Mon, Nov 20, 2023 at 01:38:06PM +0100, Lukas Funke wrote:
> Hi Russel,
>
> On 20.11.2023 10:51, Russell King (Oracle) wrote:
> > On Mon, Nov 20, 2023 at 10:32:54AM +0100, Lukas Funke wrote:
> > > From: Lukas Funke <[email protected]>
> > >
> > > When there is no driver associated with the phydev, there will be a
> > > nullptr access. The commit checks if the phydev driver is set before
> > > access.
> >
> > What's the call path that we encounter a NULL drv pointer?
>
>
> The patch is a bit older and the path is reconstructed from my memory:
>
> macb_phylink_connect -> phylink_of_phy_connect -> of_phy_connect ->
> phy_connect_direct -> phy_request_interrupt
>
> It happend when we used the Xilinx gmii2rgmii phy driver. We did a
> missconfiguration in the dt and bumped into the nullpointer exception. Since
> other functions like phy_aneg_done() also check for driver existence I
> thought it would be a good addition.
So how does this happen in the path you indicate?
phy_connect_direct() calls phy_attach_direct() before calling
phy_request_interrupt(). If phy_attach_direct() needs to succeed for
us to get to call phy_request_interrupt().
phy_attach_direct() checks to see whether the phydev is bound to a
driver. If it isn't, it binds it to the appropriate genphy driver.
As part of that binding, phydev->drv is guaranteed to be set
(by phy_probe(), which will be called via d->driver->probe() if
using one of the genphy drivers.
You mention using the gmii2rgmii driver, which does mess with
phydev->drv, but I can't see a way in that driver where we would
end up with a NULL pointer there.
I think a bit mroe information is needed to describe how this comes
about - and that needs to go in the commit message, so the reason
for this patch is properly documented.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!