Hi Kangjie, On Mon, Mar 11, 2019 at 8:01 AM Kangjie Lu <kjlu@xxxxxxx> wrote: > In case of_get_phy_mode fails, the fix returns NULL to avoid > the NULL pointer dereference. > > Signed-off-by: Kangjie Lu <kjlu@xxxxxxx> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/sh_eth.c > +++ b/drivers/net/ethernet/renesas/sh_eth.c > @@ -3187,6 +3187,8 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev) > return NULL; > > pdata->phy_interface = of_get_phy_mode(np); > + if (unlikely(!pdata->phy_interface)) > + return NULL; of_get_phy_mode() does not return a pointer, but a phy mode index. Hence zero is a valid value. So I'm wondering where is the potential NULL pointer dereference? Perhaps we do need to check for negative error codes? Or is this already handled later, by {of_,}phy_connect()? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds