Skip to content

Commit c5a9993

Browse files
bebarinodavem330
authored andcommitted
ks8851: Fix mutex deadlock in ks8851_net_stop()
There is a potential deadlock scenario when the ks8851 driver is removed. The interrupt handler schedules a workqueue which acquires a mutex that ks8851_net_stop() also acquires before flushing the workqueue. Previously lockdep wouldn't be able to find this problem but now that it has the support we can trigger this lockdep warning by rmmoding the driver after an ifconfig up. Fix the possible deadlock by disabling the interrupts in the chip and then release the lock across the workqueue flushing. The mutex is only there to proect the registers anyway so this should be ok. ======================================================= [ INFO: possible circular locking dependency detected ] 3.0.21-00021-g8b33780-dirty raspberrypi#2911 ------------------------------------------------------- rmmod/125 is trying to acquire lock: ((&ks->irq_work)){+.+...}, at: [<c019e0b8>] flush_work+0x0/0xac but task is already holding lock: (&ks->lock){+.+...}, at: [<bf00b850>] ks8851_net_stop+0x64/0x138 [ks8851] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> raspberrypi#1 (&ks->lock){+.+...}: [<c01b89c8>] __lock_acquire+0x940/0x9f8 [<c01b9058>] lock_acquire+0x10c/0x130 [<c083dbec>] mutex_lock_nested+0x68/0x3dc [<bf00bd48>] ks8851_irq_work+0x24/0x46c [ks8851] [<c019c580>] process_one_work+0x2d8/0x518 [<c019cb98>] worker_thread+0x220/0x3a0 [<c01a2ad4>] kthread+0x88/0x94 [<c0107008>] kernel_thread_exit+0x0/0x8 -> #0 ((&ks->irq_work)){+.+...}: [<c01b7984>] validate_chain+0x914/0x1018 [<c01b89c8>] __lock_acquire+0x940/0x9f8 [<c01b9058>] lock_acquire+0x10c/0x130 [<c019e104>] flush_work+0x4c/0xac [<bf00b858>] ks8851_net_stop+0x6c/0x138 [ks8851] [<c06b209c>] __dev_close_many+0x98/0xcc [<c06b2174>] dev_close_many+0x68/0xd0 [<c06b22ec>] rollback_registered_many+0xcc/0x2b8 [<c06b2554>] rollback_registered+0x28/0x34 [<c06b25b8>] unregister_netdevice_queue+0x58/0x7c [<c06b25f4>] unregister_netdev+0x18/0x20 [<bf00c1f4>] ks8851_remove+0x64/0xb4 [ks8851] [<c049ddf0>] spi_drv_remove+0x18/0x1c [<c0468e98>] __device_release_driver+0x7c/0xbc [<c0468f64>] driver_detach+0x8c/0xb4 [<c0467f00>] bus_remove_driver+0xb8/0xe8 [<c01c1d20>] sys_delete_module+0x1e8/0x27c [<c0105ec0>] ret_fast_syscall+0x0/0x3c other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&ks->lock); lock((&ks->irq_work)); lock(&ks->lock); lock((&ks->irq_work)); *** DEADLOCK *** 4 locks held by rmmod/125: #0: (&__lockdep_no_validate__){+.+.+.}, at: [<c0468f44>] driver_detach+0x6c/0xb4 raspberrypi#1: (&__lockdep_no_validate__){+.+.+.}, at: [<c0468f50>] driver_detach+0x78/0xb4 raspberrypi#2: (rtnl_mutex){+.+.+.}, at: [<c06b25e8>] unregister_netdev+0xc/0x20 raspberrypi#3: (&ks->lock){+.+...}, at: [<bf00b850>] ks8851_net_stop+0x64/0x138 [ks8851] Cc: Ben Dooks <[email protected]> Signed-off-by: Stephen Boyd <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 3adadc0 commit c5a9993

File tree

1 file changed

+6
-5
lines changed

1 file changed

+6
-5
lines changed

drivers/net/ethernet/micrel/ks8851.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -889,16 +889,17 @@ static int ks8851_net_stop(struct net_device *dev)
889889
netif_stop_queue(dev);
890890

891891
mutex_lock(&ks->lock);
892+
/* turn off the IRQs and ack any outstanding */
893+
ks8851_wrreg16(ks, KS_IER, 0x0000);
894+
ks8851_wrreg16(ks, KS_ISR, 0xffff);
895+
mutex_unlock(&ks->lock);
892896

893897
/* stop any outstanding work */
894898
flush_work(&ks->irq_work);
895899
flush_work(&ks->tx_work);
896900
flush_work(&ks->rxctrl_work);
897901

898-
/* turn off the IRQs and ack any outstanding */
899-
ks8851_wrreg16(ks, KS_IER, 0x0000);
900-
ks8851_wrreg16(ks, KS_ISR, 0xffff);
901-
902+
mutex_lock(&ks->lock);
902903
/* shutdown RX process */
903904
ks8851_wrreg16(ks, KS_RXCR1, 0x0000);
904905

@@ -907,6 +908,7 @@ static int ks8851_net_stop(struct net_device *dev)
907908

908909
/* set powermode to soft power down to save power */
909910
ks8851_set_powermode(ks, PMECR_PM_SOFTDOWN);
911+
mutex_unlock(&ks->lock);
910912

911913
/* ensure any queued tx buffers are dumped */
912914
while (!skb_queue_empty(&ks->txq)) {
@@ -918,7 +920,6 @@ static int ks8851_net_stop(struct net_device *dev)
918920
dev_kfree_skb(txb);
919921
}
920922

921-
mutex_unlock(&ks->lock);
922923
return 0;
923924
}
924925

0 commit comments

Comments
 (0)