> -----Original Message----- > From: maowenan > Sent: Wednesday, February 08, 2017 9:56 AM > To: 'Dan Carpenter' > Cc: 'kernel-janitors@xxxxxxxxxxxxxxx' > Subject: RE: [bug report] net: phy: Fix lack of reference count on PHY driver > > > > > -----Original Message----- > > From: maowenan > > Sent: Wednesday, February 08, 2017 9:28 AM > > To: 'Dan Carpenter' > > Cc: kernel-janitors@xxxxxxxxxxxxxxx > > Subject: RE: [bug report] net: phy: Fix lack of reference count on PHY > > driver > > > > Thank you very much , there should be pointer checked before dereference. > > > > > > > -----Original Message----- > > > From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx] > > > Sent: Tuesday, February 07, 2017 6:50 PM > > > To: maowenan > > > Cc: kernel-janitors@xxxxxxxxxxxxxxx > > > Subject: [bug report] net: phy: Fix lack of reference count on PHY > > > driver > > > > > > Hello Mao Wenan, > > > > > > This is a semi-automatic email about new static checker warnings. > > > > > > The patch cafe8df8b9bc: "net: phy: Fix lack of reference count on PHY > driver" > > > from Jan 31, 2017, leads to the following Smatch complaint: > > > > > > drivers/net/phy/phy_device.c:933 phy_attach_direct() > > > warn: variable dereferenced before check 'd->driver' (see line > > > 923) > > > > > > drivers/net/phy/phy_device.c > > > 922 > > > 923 if (!try_module_get(d->driver->owner)) { > > > ^^^^^^^^^^^ Patch > introduces > > a new > > > dereference. > > > > > > 924 dev_err(&dev->dev, "failed to get the device driver > > > module\n"); > > > 925 return -EIO; > > > 926 } > > > 927 > > > 928 get_device(d); > > > 929 > > > 930 /* Assume that if there is no driver, that it doesn't > > > 931 * exist, and we should use the genphy driver. > > > 932 */ > > > 933 if (!d->driver) { > > > ^^^^^^^^^ > > > Existing code assumed d->driver could be NULL. > > > > > > 934 if (phydev->is_c45) > > > 935 d->driver = > > > > > > regards, > > > dan carpenter > > Hi dan, > > Is it OK if I put below judgment after if (!d->driver) {......} ? > > if (!try_module_get(d->driver->owner)) { > dev_err(&dev->dev, "failed to get the device driver module\n"); > return -EIO; > } > ================================================ > if (ndev_owner != bus->owner && !try_module_get(bus->owner)) { > dev_err(&dev->dev, "failed to get the bus module\n"); > return -EIO; > } > > if (!try_module_get(d->driver->owner)) { > dev_err(&dev->dev, "failed to get the device driver module\n"); > return -EIO; > } > > get_device(d); > > /* Assume that if there is no driver, that it doesn't > * exist, and we should use the genphy driver. > */ > if (!d->driver) { > if (phydev->is_c45) > d->driver = > &genphy_driver[GENPHY_DRV_10G].mdiodrv.driver; > else > d->driver = > &genphy_driver[GENPHY_DRV_1G].mdiodrv.driver; > > err = d->driver->probe(d); > if (err >= 0) > err = device_bind_driver(d); > > if (err) > goto error; > } > > ======================》》》》》》》》》》》》》》》》》》》》》》》》》》》》》》》》》》 > if (ndev_owner != bus->owner && !try_module_get(bus->owner)) { > dev_err(&dev->dev, "failed to get the bus module\n"); > return -EIO; > } > > get_device(d); > > /* Assume that if there is no driver, that it doesn't > * exist, and we should use the genphy driver. > */ > if (!d->driver) { > if (phydev->is_c45) > d->driver = > &genphy_driver[GENPHY_DRV_10G].mdiodrv.driver; > else > d->driver = > &genphy_driver[GENPHY_DRV_1G].mdiodrv.driver; > > err = d->driver->probe(d); > if (err >= 0) > err = device_bind_driver(d); > > if (err) > goto error; > } > > if (!try_module_get(d->driver->owner)) { > dev_err(&dev->dev, "failed to get the device driver module\n"); > return -EIO; > } Hi dan, Sorry for trouble, I find another issue about this patch, if try_module_get(d->driver->owner) failure, it can't return -EIO, it must firstly call module_put(bus->owner) to release bus owner reference. I will refine the codes and repost patch, thanks again. ?韬{.n?????%??檩??w?{.n???罐??+h???n?■???h?璀?｛?夸z罐?+€?zf＂?????i?????_璁?:+v??撸?
