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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 5 Feb 2014 14:09:54 -0800 From: Nithin Nayak Sujir <nsujir@...adcom.com> To: David Vrabel <david.vrabel@...rix.com>, <netdev@...r.kernel.org> CC: Michael Chan <mchan@...adcom.com> Subject: Re: [PATCHv1 net] tg3: fix deadlock in tg3_change_mtu() On 02/04/2014 11:01 AM, David Vrabel wrote: > 5780 cards cannot have jumbo frames and TSO enabled together. When > jumbo frames are enabled by setting the MTU, the TSO feature must be > cleared. This is done indirectly by calling netdev_update_features() > which will call tg3_fix_features() to actually clear the flags. > > netdev_update_features() will also trigger a new netlink message for > the feature change event which will result in a call to > tg3_get_stats64() which deadlocks on the tg3 lock. > > Fix this by dropping the tg3 lock while calling > netdev_update_features(). This is safe as the device is down and the > hardware is stopped. > > Signed-off-by: David Vrabel <david.vrabel@...rix.com> > --- > drivers/net/ethernet/broadcom/tg3.c | 18 +++++++++++------- > 1 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index e2ca03e..0b344c0 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -14077,17 +14077,13 @@ static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp, > dev->mtu = new_mtu; > > if (new_mtu > ETH_DATA_LEN) { > - if (tg3_flag(tp, 5780_CLASS)) { > - netdev_update_features(dev); > + if (tg3_flag(tp, 5780_CLASS)) > tg3_flag_clear(tp, TSO_CAPABLE); > - } else { > + else > tg3_flag_set(tp, JUMBO_RING_ENABLE); > - } > } else { > - if (tg3_flag(tp, 5780_CLASS)) { > + if (tg3_flag(tp, 5780_CLASS)) > tg3_flag_set(tp, TSO_CAPABLE); > - netdev_update_features(dev); > - } > tg3_flag_clear(tp, JUMBO_RING_ENABLE); > } > } > @@ -14119,6 +14115,14 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu) > > tg3_set_mtu(dev, tp, new_mtu); > > + tg3_full_unlock(tp); > + > + /* Adjusting MTU may add/remove TSO feature. */ > + if (tg3_flag(tp, 5780_CLASS)) > + netdev_update_features(dev); > + > + tg3_full_lock(tp, 0); > + > /* Reset PHY, otherwise the read DMA engine will be in a mode that > * breaks all requests to 256 bytes. > */ > David, Rather than doing it this way, I think it's sufficient to move the call to tg3_set_mtu() between tg3_netif_stop() and tg3_full_lock(). Before tg3 started using set_bit() for flags, tg3_set_mtu() needed to be under a lock, but that's not the case anymore. Can you try this patch? diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index e2ca03e..0bb79b8 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -14113,12 +14113,12 @@ static int tg3_change_mtu(struct net_device *dev, int new_mtu) tg3_netif_stop(tp); + tg3_set_mtu(dev, tp, new_mtu); + tg3_full_lock(tp, 1); tg3_halt(tp, RESET_KIND_SHUTDOWN, 1); - tg3_set_mtu(dev, tp, new_mtu); - /* Reset PHY, otherwise the read DMA engine will be in a mode that * breaks all requests to 256 bytes. */ Nithin. -- 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