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
| ||
|
Date: Tue, 9 Feb 2010 18:53:01 -0700 From: Grant Likely <grant.likely@...retlab.ca> To: John Linn <John.Linn@...inx.com> Cc: John Williams <john.williams@...alogix.com>, netdev@...r.kernel.org, linuxppc-dev@...abs.org, jgarzik@...ox.com, jwboyer@...ux.vnet.ibm.com, Sadanand Mutyala <sadanan@...inx.com> Subject: Re: [PATCH] [V3] net: emaclite: adding MDIO and phy lib support On Tue, Feb 9, 2010 at 4:00 PM, John Linn <John.Linn@...inx.com> wrote: >> -----Original Message----- >> From: John Williams [mailto:john.williams@...alogix.com] >> Sent: Tuesday, February 09, 2010 3:30 PM >> To: John Linn >> Cc: netdev@...r.kernel.org; linuxppc-dev@...abs.org; jgarzik@...ox.com; grant.likely@...retlab.ca; >> jwboyer@...ux.vnet.ibm.com; Sadanand Mutyala >> Subject: Re: [PATCH] [V3] net: emaclite: adding MDIO and phy lib support >> >> Hi John, >> >> Sorry If I'm painting bike-sheds here, just one tiny tweak might be in >> order to standardise your mutex_unlock exit path: >> >> > +static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg) >> > +{ >> > + struct net_local *lp = bus->priv; >> > + u32 ctrl_reg; >> > + u32 rc; >> > + >> > + mutex_lock(&lp->mdio_mutex); >> > + >> > + if (xemaclite_mdio_wait(lp)) { >> > + mutex_unlock(&lp->mdio_mutex); >> > + return -ETIMEDOUT; >> > + } >> >> [snip] >> >> >> > + if (xemaclite_mdio_wait(lp)) { >> > + mutex_unlock(&lp->mdio_mutex); >> > + return -ETIMEDOUT; >> > + } >> >> [snip] >> >> >> > + dev_dbg(&lp->ndev->dev, >> > + "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n", >> > + phy_id, reg, rc); >> > + >> > + return rc; >> >> Can this be better expressed like this: >> >> my_func() { >> mutex_lock() >> .. >> >> if(some error) { >> rc=-ETIMEDOUT; >> goto out_unlock; >> } >> ... >> >> /* success path */ >> rc=0; >> .. >> out_unlock: >> mutex_unlock() >> return rc; >> } >> >> >> Is this style still favoured in driver exit paths? > > It looks to me like the mutex is not needed in the driver mdio functions as there's a mutex in the mdiobus functions already. Yes, you're correct, but you still need to protect against direct calls to the read/write routines from within the driver. But you can probably use the mdio_lock mutex for this. g. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists