[PATCH] esas2r: Fix possible sleep-in-atomic bugs in esas2r_check_adapter
The driver may sleep in the interrupt handler. The function call path is: esas2r_adapter_tasklet (interrupt handler) esas2r_do_tasklet_tasks esas2r_handle_chip_rst_during_tasklet esas2r_check_adapter schedule_timeout_interruptible To fix it, schedule_timeout_uninterruptible is replaced with mdelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/scsi/esas2r/esas2r_init.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/esas2r/esas2r_init.c b/drivers/scsi/esas2r/esas2r_init.c index 5b14dd2..0b9f547 100644 --- a/drivers/scsi/esas2r/esas2r_init.c +++ b/drivers/scsi/esas2r/esas2r_init.c @@ -1068,7 +1068,7 @@ bool esas2r_check_adapter(struct esas2r_adapter *a) break; } - schedule_timeout_interruptible(msecs_to_jiffies(100)); + mdelay(100); if ((jiffies_to_msecs(jiffies) - starttime) > 18) { esas2r_hdebug("FW ready TMO"); @@ -1091,7 +1091,7 @@ bool esas2r_check_adapter(struct esas2r_adapter *a) break; } - schedule_timeout_interruptible(msecs_to_jiffies(50)); + mdelay(50); if ((jiffies_to_msecs(jiffies) - starttime) > 3000) { esas2r_hdebug("timeout waiting for interface down"); @@ -1180,7 +1180,7 @@ bool esas2r_check_adapter(struct esas2r_adapter *a) break; } - schedule_timeout_interruptible(msecs_to_jiffies(100)); + mdelay(100); if ((jiffies_to_msecs(jiffies) - starttime) > 3000) { esas2r_hdebug( -- 1.7.9.5
[BUG] drivers/scsi/esas2r: a possible sleep-in-atomic bug in esas2r_nvram_read_direct
The driver may sleep in the interrupt handler. The function call path is: esas2r_adapter_tasklet (interrupt handler) esas2r_do_tasklet_tasks esas2r_handle_chip_rst_during_tasklet esas2r_init_adapter_hw esas2r_nvram_read_direct down_interruptible --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[PATCH] esas2r: Fix a possible sleep-in-atomic bug in esas2r_flash_access
The driver may sleep in the interrupt handler. The function call path is: esas2r_adapter_tasklet (interrupt handler) esas2r_do_tasklet_tasks esas2r_handle_chip_rst_during_tasklet esas2r_init_adapter_hw esas2r_nvram_read_direct esas2r_read_flash_block esas2r_flash_access schedule_timeout_interruptible --> may sleep To fix it, schedule_timeout_uninterruptible is replaced with mdelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/scsi/esas2r/esas2r_flash.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/esas2r/esas2r_flash.c b/drivers/scsi/esas2r/esas2r_flash.c index 7bd376d..9b3da4c 100644 --- a/drivers/scsi/esas2r/esas2r_flash.c +++ b/drivers/scsi/esas2r/esas2r_flash.c @@ -965,7 +965,7 @@ static bool esas2r_flash_access(struct esas2r_adapter *a, u32 function) break; } - schedule_timeout_interruptible(msecs_to_jiffies(100)); + mdelay(100); if ((jiffies_to_msecs(jiffies) - starttime) > timeout) { /* -- 1.7.9.5
[PATCH] esas2r: Fix a possible sleep-in-atomic bug in esas2r_wait_request
The driver may sleep in the interrupt handler. The function call path is: esas2r_adapter_tasklet (interrupt handler) esas2r_do_tasklet_tasks esas2r_handle_chip_rst_during_tasklet esas2r_init_adapter_hw esas2r_init_msgs esas2r_wait_request schedule_timeout_interruptible --> may sleep To fix it, schedule_timeout_uninterruptible is replaced with mdelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/scsi/esas2r/esas2r_main.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c index 4eb1430..4cd8f79 100644 --- a/drivers/scsi/esas2r/esas2r_main.c +++ b/drivers/scsi/esas2r/esas2r_main.c @@ -1307,7 +1307,7 @@ void esas2r_wait_request(struct esas2r_adapter *a, struct esas2r_request *rq) if (rq->req_stat != RS_STARTED) break; - schedule_timeout_interruptible(msecs_to_jiffies(100)); + mdelay(100); if ((jiffies_to_msecs(jiffies) - starttime) > timeout) { esas2r_hdebug("request TMO"); -- 1.7.9.5
[BUG] drivers/input/misc/pcap: a possible sleep-in-atomic bug in pcap_keys_handler
According to drivers/input/misc/pcap_keys.c, the kernel module may sleep in the interrupt handler. The function call path is: pcap_keys_handler (interrupt handler) ezx_pcap_read mutex_lock --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] skge: a possible sleep-in-atomic bug in skge_remove
According to drivers/net/ethernet/marvell/skge.c, the driver may sleep under a spinlock. The function call path is: skge_remove (acquire the spinlock) free_irq --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[PATCH] hippi: Fix a Fix a possible sleep-in-atomic bug in rr_close
The driver may sleep under a spinlock. The function call path is: rr_close (acquire the spinlock) free_irq --> may sleep To fix it, free_irq is moved to the place without holding the spinlock. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/net/hippi/rrunner.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c index 8483f03..1ab97d9 100644 --- a/drivers/net/hippi/rrunner.c +++ b/drivers/net/hippi/rrunner.c @@ -1379,8 +1379,8 @@ static int rr_close(struct net_device *dev) rrpriv->info_dma); rrpriv->info = NULL; - free_irq(pdev->irq, dev); spin_unlock_irqrestore(&rrpriv->lock, flags); + free_irq(pdev->irq, dev); return 0; } -- 1.7.9.5
[BUG] sbni: a possible sleep-in-atomic bug in sbni_close
According to drivers/net/wan/sbni.c, the driver may sleep under a spinlock. The function call path is: sbni_close (acquire the spinlock) free_irq --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] wl3501: a possible sleep-in-atomic bug in wl3501_reset
According to drivers/net/wireless/wl3501_cs.c, the driver may sleep under a spinlock. The function call path is: wl3501_reset (acquire the spinlock) free_irq --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[PATCH] mac80211_hwsim: Fix a possible sleep-in-atomic bug in hwsim_get_radio_nl
The driver may sleep under a spinlock. The function call path is: hwsim_get_radio_nl (acquire the spinlock) nlmsg_new(GFP_KERNEL) --> may sleep To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/mac80211_hwsim.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c index 10b075a..f2ebf4a 100644 --- a/drivers/net/wireless/mac80211_hwsim.c +++ b/drivers/net/wireless/mac80211_hwsim.c @@ -3215,7 +3215,7 @@ static int hwsim_get_radio_nl(struct sk_buff *msg, struct genl_info *info) if (!net_eq(wiphy_net(data->hw->wiphy), genl_info_net(info))) continue; - skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC); if (!skb) { res = -ENOMEM; goto out_err; -- 1.7.9.5
[BUG] atmel_ssc_dai: a possible sleep-in-atomic bug in atmel_ssc_shutdown
According to sound/soc/atmel/atmel_ssc_dai.c, the driver may sleep under a spinlock. The function call path is: atmel_ssc_shutdown (acquire the spinlock) free_irq --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] vme_ca91cx42: a possible sleep-in-atomic bug in ca91cx42_master_set
According to drivers/vme/bridges/vme_ca91cx42.c, the driver may sleep under a spinlock. The function call path is: ca91cx42_master_set (acquire the spinlock) ca91cx42_alloc_resource ioremap_nocache --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[PATCH] vme: Fix a possible sleep-in-atomic bug in vme_tsi148
The driver may sleep under a spinlock. The function call path is: tsi148_master_write \ tsi148_master_read (acquire the spinlock) vme_register_error_handler kmalloc(GFP_KERNEL) --> may sleep To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/vme/vme.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c index 8124622..92500f6 100644 --- a/drivers/vme/vme.c +++ b/drivers/vme/vme.c @@ -1290,7 +1290,7 @@ struct vme_error_handler *vme_register_error_handler( { struct vme_error_handler *handler; - handler = kmalloc(sizeof(*handler), GFP_KERNEL); + handler = kmalloc(sizeof(*handler), GFP_ATOMIC); if (!handler) return NULL; -- 1.7.9.5
[PATCH] rtl8188eu: Fix a possible sleep-in-atomic bug in set_tx_beacon_cmd
The driver may sleep under a spinlock. The function call path is: update_beacon (acquire the spinlock) update_BCNTIM set_tx_beacon_cmd kzalloc(GFP_KERNEL) --> may sleep kmemdup(GFP_KERNEL) --> may sleep To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/staging/rtl8188eu/core/rtw_mlme_ext.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c index d73e9bd..bcb6919 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c @@ -5395,14 +5395,14 @@ u8 set_tx_beacon_cmd(struct adapter *padapter) int len_diff = 0; - ph2c = kzalloc(sizeof(struct cmd_obj), GFP_KERNEL); + ph2c = kzalloc(sizeof(struct cmd_obj), GFP_ATOMIC); if (!ph2c) { res = _FAIL; goto exit; } ptxBeacon_parm = kmemdup(&(pmlmeinfo->network), - sizeof(struct wlan_bssid_ex), GFP_KERNEL); + sizeof(struct wlan_bssid_ex), GFP_ATOMIC); if (ptxBeacon_parm == NULL) { kfree(ph2c); res = _FAIL; -- 1.7.9.5
[PATCH] tty/isicom: Fix a possible sleep-in-atomic bug in WaitTillCardIsFree
The driver may sleep under a spinlock. The function call paths are: isicom_activate (acquire the spinlock) isicom_setup_board drop_dtr_rts WaitTillCardIsFree msleep --> may sleep isicom_set_termios isicom_config_port drop_dtr WaitTillCardIsFree msleep --> may sleep isicom_tiocmset drop_dtr WaitTillCardIsFree msleep --> may sleep Though "in_atomic" is used to check atomic context, but it is not recommended to use in driver code (see include/linux/preempt.h). To fix it, only using mdelay instead. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/tty/isicom.c |6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/tty/isicom.c b/drivers/tty/isicom.c index 015686f..bdd3027 100644 --- a/drivers/tty/isicom.c +++ b/drivers/tty/isicom.c @@ -219,13 +219,9 @@ struct isi_port { static int WaitTillCardIsFree(unsigned long base) { unsigned int count = 0; - unsigned int a = in_atomic(); /* do we run under spinlock? */ while (!(inw(base + 0xe) & 0x1) && count++ < 100) - if (a) - mdelay(1); - else - msleep(1); + mdelay(1); return !(inw(base + 0xe) & 0x1); } -- 1.7.9.5
[PATCH] arcmsr: Fix possible sleep-in-atomic bugs in arcmsr_queue_command
From: Jia-Ju Bai The driver may sleep under a spinlock, and the function call paths are: arcmsr_queue_command(acquire the spinlock) arcmsr_queue_command_lck arcmsr_handle_virtual_command arcmsr_iop_message_xfer arcmsr_iop_parking arcmsr_stop_adapter_bgrb arcmsr_hbaA_stop_bgrb arcmsr_hbaA_wait_msgint_ready msleep --> may sleep arcmsr_queue_command(acquire the spinlock) arcmsr_queue_command_lck arcmsr_handle_virtual_command arcmsr_iop_message_xfer arcmsr_iop_parking arcmsr_stop_adapter_bgrb arcmsr_hbaB_stop_bgrb arcmsr_hbaB_wait_msgint_ready msleep --> may sleep arcmsr_queue_command(acquire the spinlock) arcmsr_queue_command_lck arcmsr_handle_virtual_command arcmsr_iop_message_xfer arcmsr_iop_parking arcmsr_stop_adapter_bgrb arcmsr_hbaC_stop_bgrb arcmsr_hbaC_wait_msgint_ready msleep --> may sleep arcmsr_queue_command(acquire the spinlock) arcmsr_queue_command_lck arcmsr_handle_virtual_command arcmsr_iop_message_xfer arcmsr_iop_parking arcmsr_stop_adapter_bgrb arcmsr_hbaD_stop_bgrb arcmsr_hbaD_wait_msgint_ready msleep --> may sleep To fix them, msleep is replaced with mdelay. These bugs are found by my static analysis tool and my code review. Signed-off-by: Jia-Ju Bai --- drivers/scsi/arcmsr/arcmsr_hba.c |8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c index af032c4..31d94bd 100644 --- a/drivers/scsi/arcmsr/arcmsr_hba.c +++ b/drivers/scsi/arcmsr/arcmsr_hba.c @@ -347,7 +347,7 @@ static uint8_t arcmsr_hbaA_wait_msgint_ready(struct AdapterControlBlock *acb) ®->outbound_intstatus); return true; } - msleep(10); + mdelay(10); } /* max 20 seconds */ return false; @@ -367,7 +367,7 @@ static uint8_t arcmsr_hbaB_wait_msgint_ready(struct AdapterControlBlock *acb) reg->drv2iop_doorbell); return true; } - msleep(10); + mdelay(10); } /* max 20 seconds */ return false; @@ -385,7 +385,7 @@ static uint8_t arcmsr_hbaC_wait_msgint_ready(struct AdapterControlBlock *pACB) &phbcmu->outbound_doorbell_clear); /*clear interrupt*/ return true; } - msleep(10); + mdelay(10); } /* max 20 seconds */ return false; @@ -403,7 +403,7 @@ static bool arcmsr_hbaD_wait_msgint_ready(struct AdapterControlBlock *pACB) reg->outbound_doorbell); return true; } - msleep(10); + mdelay(10); } /* max 20 seconds */ return false; } -- 1.7.9.5
[PATCH 2/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_save_request
The driver may sleep under a spinlock. The function call path is: bdisp_device_run (acquire the spinlock) bdisp_hw_update bdisp_hw_save_request devm_kzalloc(GFP_KERNEL) --> may sleep To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c b/drivers/media/platform/sti/bdisp/bdisp-hw.c index 4b62ceb..7b45b43 100644 --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c @@ -1064,7 +1064,7 @@ static void bdisp_hw_save_request(struct bdisp_ctx *ctx) if (!copy_node[i]) { copy_node[i] = devm_kzalloc(ctx->bdisp_dev->dev, sizeof(*copy_node[i]), - GFP_KERNEL); + GFP_ATOMIC); if (!copy_node[i]) return; } -- 1.7.9.5
[PATCH 1/2] bdisp: Fix a possible sleep-in-atomic bug in bdisp_hw_reset
The driver may sleep under a spinlock. The function call path is: bdisp_device_run (acquire the spinlock) bdisp_hw_reset msleep --> may sleep To fix it, msleep is replaced with mdelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/media/platform/sti/bdisp/bdisp-hw.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/sti/bdisp/bdisp-hw.c b/drivers/media/platform/sti/bdisp/bdisp-hw.c index b7892f3..4b62ceb 100644 --- a/drivers/media/platform/sti/bdisp/bdisp-hw.c +++ b/drivers/media/platform/sti/bdisp/bdisp-hw.c @@ -382,7 +382,7 @@ int bdisp_hw_reset(struct bdisp_dev *bdisp) for (i = 0; i < POLL_RST_MAX; i++) { if (readl(bdisp->regs + BLT_STA1) & BLT_STA1_IDLE) break; - msleep(POLL_RST_DELAY_MS); + mdelay(POLL_RST_DELAY_MS); } if (i == POLL_RST_MAX) dev_err(bdisp->dev, "Reset timeout\n"); -- 1.7.9.5
[BUG] haswell: a possible sleep-in-atomic bug in hsw_irq_thread
According to sound/soc/intel/haswell/sst-haswell-ipc.c, the driver may sleep under a spinlock. The function call path is: hsw_irq_thread (acquire the spinlock) hsw_process_notification hsw_log_message mutex_lock --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] soc/sti: a possible sleep-in-atomic bug in uni_player_ctl_iec958_put
According to sound/soc/sti/uniperif_player.c, the driver may sleep under a spinlock. The function call path is: uni_player_ctl_iec958_put (acquire the spinlock) uni_player_set_channel_status mutex_lock --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] scsi/qla2xxx: a possible sleep-in-atomic bug in qlt_get_tag
The driver may sleep under a spinlock. The function call paths are: qlt_handle_abts_recv_work (acquire the spinlock) qlt_response_pkt_all_vps qlt_response_pkt qlt_handle_cmd_for_atio qlt_get_tag percpu_ida_alloc --> may sleep qla82xx_msix_rsp_q (acquire the spinlock) qla24xx_process_response_queue qlt_handle_abts_recv qlt_response_pkt_all_vps qlt_response_pkt qlt_handle_cmd_for_atio qlt_get_tag percpu_ida_alloc --> may sleep-in-atomic qla24xx_intr_handler (acquire the spinlock) qla24xx_process_response_queue qlt_handle_abts_recv qlt_response_pkt qlt_handle_cmd_for_atio qlt_get_tag percpu_ida_alloc --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] b44: two possible sleep-in-atomic bugs in b44_set_link_ksettings and b44_ioctl
The driver may sleep under a spinlock. The function call paths are: b44_set_link_ksettings (acquire the spinlock) phy_ethtool_ksettings_set phy_start_aneg phy_start_aneg_priv mutex_lock --> may sleep b44_ioctl (acquire the spinlock) phy_mii_ioctl mdiobus_read mutex_lock --> may sleep I do not find a good way to fix them, so I only report. These possible bugs are found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] renesas/sh_eth: two possible sleep-in-atomic bugs in sh_eth_set_link_ksettings and sh_eth_nway_reset
Accoring to drivers/net/ethernet/renesas/sh_eth.c, the driver may sleep under a spinlock. The function call paths are: sh_eth_set_link_ksettings (acquire the spinlock) phy_ethtool_ksettings_set phy_start_aneg phy_start_aneg_priv mutex_lock --> may sleep sh_eth_nway_reset (acquire the spinlock) phy_start_aneg phy_start_aneg_priv mutex_lock --> may sleep I do not find a good way to fix them, so I only report. These possible bugs are found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] renesas/ravb: two possible sleep-in-atomic bugs in ravb_set_link_ksettings and ravb_nway_reset
Accoring to drivers/net/ethernet/renesas/ravb_main.c, the driver may sleep under a spinlock. The function call paths are: ravb_set_link_ksettings (acquire the spinlock) phy_ethtool_ksettings_set phy_start_aneg phy_start_aneg_priv mutex_lock --> may sleep ravb_nway_reset (acquire the spinlock) phy_start_aneg phy_start_aneg_priv mutex_lock --> may sleep I do not find a good way to fix them, so I only report. These possible bugs are found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
Re: [BUG] scsi/qla2xxx: a possible sleep-in-atomic bug in qlt_get_tag
On 2017/12/13 12:42, James Bottomley wrote: On Wed, 2017-12-13 at 11:18 +0800, Jia-Ju Bai wrote: The driver may sleep under a spinlock. The function call paths are: qlt_handle_abts_recv_work (acquire the spinlock) qlt_response_pkt_all_vps qlt_response_pkt qlt_handle_cmd_for_atio qlt_get_tag percpu_ida_alloc --> may sleep qla82xx_msix_rsp_q (acquire the spinlock) qla24xx_process_response_queue qlt_handle_abts_recv qlt_response_pkt_all_vps qlt_response_pkt qlt_handle_cmd_for_atio qlt_get_tag percpu_ida_alloc --> may sleep-in-atomic qla24xx_intr_handler (acquire the spinlock) qla24xx_process_response_queue qlt_handle_abts_recv qlt_response_pkt qlt_handle_cmd_for_atio qlt_get_tag percpu_ida_alloc --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. The report is incorrect: percpu_ida_alloc with state==TASK_RUNNING is atomic (and interrupt) safe which appears to be the case here. James Thanks for your reply :) I have checked the definition of percpu_ida_alloc, and I think you are right. Sorry for my incorrect bug report. Thanks, Jia-Ju Bai
Re: [BUG] skge: a possible sleep-in-atomic bug in skge_remove
On 2017/12/13 13:18, Stephen Hemminger wrote: On Tue, 12 Dec 2017 20:57:01 -0500 (EST) David Miller wrote: From: Stephen Hemminger Date: Tue, 12 Dec 2017 10:22:40 -0800 On Tue, 12 Dec 2017 08:34:45 -0500 (EST) David Miller wrote: From: Jia-Ju Bai Date: Tue, 12 Dec 2017 16:38:12 +0800 According to drivers/net/ethernet/marvell/skge.c, the driver may sleep under a spinlock. The function call path is: skge_remove (acquire the spinlock) free_irq --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. This was added by: commit a9e9fd7182332d0cf5f3e601df3e71dd431b70d7 Author: Stephen Hemminger Date: Tue Sep 27 13:41:37 2011 -0400 skge: handle irq better on single port card I think the free_irq() can be moved below the unlock. Stephen, please take a look. The IRQ was being free twice. How did you see it, I really doubt any multi-port SKGE cards still exist. He sees it by reading the code, please take a look at this and move the free_irq() out of the spin locked section since it can sleep. Thanks, I was hoping for some automated static analysis tool. This bug was found by an automated static analysis tool named DSAC, which is written by myself. Then I manually checked driver source code, and finally sent the bug report. Thanks, Jia-Ju Bai
[BUG] kaweth: a possible sleep-in-atomic bug in kaweth_start_xmit
According to drivers/net/usb/kaweth.c, the driver may sleep under a spinlock. The function call path is: kaweth_start_xmit (acquire the spinlock) kaweth_async_set_rx_mode kaweth_control kaweth_internal_control_msg usb_start_wait_urb wait_event_timeout --> may sleep usb_kill_urb --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[PATCH] qedi: Fix a possible sleep-in-atomic bug in qedi_process_tmf_resp
The driver may sleep under a spinlock. The function call path is: qedi_cpu_offline (acquire the spinlock) qedi_fp_process_cqes qedi_mtask_completion qedi_process_tmf_resp kzalloc(GFP_KERNEL) --> may sleep To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/scsi/qedi/qedi_fw.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/qedi/qedi_fw.c b/drivers/scsi/qedi/qedi_fw.c index bd302d3..20a9259 100644 --- a/drivers/scsi/qedi/qedi_fw.c +++ b/drivers/scsi/qedi/qedi_fw.c @@ -198,7 +198,7 @@ static void qedi_process_tmf_resp(struct qedi_ctx *qedi, cqe_tmp_response = &cqe->cqe_common.iscsi_hdr.tmf_response; qedi_cmd = task->dd_data; - qedi_cmd->tmf_resp_buf = kzalloc(sizeof(*resp_hdr_ptr), GFP_KERNEL); + qedi_cmd->tmf_resp_buf = kzalloc(sizeof(*resp_hdr_ptr), GFP_ATOMIC); if (!qedi_cmd->tmf_resp_buf) { QEDI_ERR(&qedi->dbg_ctx, "Failed to allocate resp buf, cid=0x%x\n", -- 1.7.9.5
[PATCH] bluecard: Fix a possible sleep-in-atomic bug in bluecard_write_wakeup
The driver may sleep in the interrupt handler. The function call path is: bluecard_interrupt (interrupt handler) bluecard_write_wakeup schedule_timeout --> may sleep To fix it, schedule_timeout is replaced with mdelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/bluetooth/bluecard_cs.c |8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c index d513ef4..82437a6 100644 --- a/drivers/bluetooth/bluecard_cs.c +++ b/drivers/bluetooth/bluecard_cs.c @@ -302,9 +302,7 @@ static void bluecard_write_wakeup(struct bluecard_info *info) } /* Wait until the command reaches the baseband */ - prepare_to_wait(&wq, &wait, TASK_INTERRUPTIBLE); - schedule_timeout(HZ/10); - finish_wait(&wq, &wait); + mdelay(100); /* Set baud on baseband */ info->ctrl_reg &= ~0x03; @@ -316,9 +314,7 @@ static void bluecard_write_wakeup(struct bluecard_info *info) outb(info->ctrl_reg, iobase + REG_CONTROL); /* Wait before the next HCI packet can be send */ - prepare_to_wait(&wq, &wait, TASK_INTERRUPTIBLE); - schedule_timeout(HZ); - finish_wait(&wq, &wait); + mdelay(1000); } if (len == skb->len) { -- 1.7.9.5
[PATCH 1/2] hp100: Fix a possible sleep-in-atomic bug in hp100_login_to_vg_hub
The driver may sleep under a spinlock. The function call path is: hp100_set_multicast_list (acquire the spinlock) hp100_login_to_vg_hub schedule_timeout_interruptible --> may sleep To fix it, schedule_timeout_interruptible is replaced with udelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/hp/hp100.c |9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/hp/hp100.c b/drivers/net/ethernet/hp/hp100.c index c8c7ad2..6addcbd 100644 --- a/drivers/net/ethernet/hp/hp100.c +++ b/drivers/net/ethernet/hp/hp100.c @@ -2636,8 +2636,7 @@ static int hp100_login_to_vg_hub(struct net_device *dev, u_short force_relogin) do { if (~(hp100_inb(VG_LAN_CFG_1) & HP100_LINK_UP_ST)) break; - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_after(time, jiffies)); /* Start an addressed training and optionally request promiscuous port */ @@ -2672,8 +2671,7 @@ static int hp100_login_to_vg_hub(struct net_device *dev, u_short force_relogin) do { if (hp100_inb(VG_LAN_CFG_1) & HP100_LINK_CABLE_ST) break; - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_before(jiffies, time)); if (time_after_eq(jiffies, time)) { @@ -2696,8 +2694,7 @@ static int hp100_login_to_vg_hub(struct net_device *dev, u_short force_relogin) #endif break; } - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_after(time, jiffies)); } -- 1.7.9.5
[PATCH 2/2] hp100: Fix a possible sleep-in-atomic bug in hp100_down_vg_link
The driver may sleep under a spinlock. The function call path is: hp100_set_multicast_list (acquire the spinlock) hp100_login_to_vg_hub hp100_down_vg_link schedule_timeout_interruptible --> may sleep To fix it, schedule_timeout_interruptible is replaced with udelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/hp/hp100.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/hp/hp100.c b/drivers/net/ethernet/hp/hp100.c index c8c7ad2..e0e6376 100644 --- a/drivers/net/ethernet/hp/hp100.c +++ b/drivers/net/ethernet/hp/hp100.c @@ -2504,8 +2504,7 @@ static int hp100_down_vg_link(struct net_device *dev) do { if (hp100_inb(VG_LAN_CFG_1) & HP100_LINK_CABLE_ST) break; - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_after(time, jiffies)); if (time_after_eq(jiffies, time)) /* no signal->no logout */ @@ -2521,8 +2520,7 @@ static int hp100_down_vg_link(struct net_device *dev) do { if (!(hp100_inb(VG_LAN_CFG_1) & HP100_LINK_UP_ST)) break; - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_after(time, jiffies)); #ifdef HP100_DEBUG @@ -2560,8 +2558,7 @@ static int hp100_down_vg_link(struct net_device *dev) do { if (!(hp100_inb(MAC_CFG_4) & HP100_MAC_SEL_ST)) break; - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_after(time, jiffies)); hp100_orb(HP100_AUTO_MODE, MAC_CFG_3); /* Autosel back on */ @@ -2572,8 +2569,7 @@ static int hp100_down_vg_link(struct net_device *dev) do { if ((hp100_inb(VG_LAN_CFG_1) & HP100_LINK_CABLE_ST) == 0) break; - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_after(time, jiffies)); if (time_before_eq(time, jiffies)) { @@ -2585,8 +2581,7 @@ static int hp100_down_vg_link(struct net_device *dev) time = jiffies + (2 * HZ); /* This seems to take a while */ do { - if (!in_interrupt()) - schedule_timeout_interruptible(1); + udelay(10); } while (time_after(time, jiffies)); return 0; -- 1.7.9.5
[PATCH 2/2] qla3xxx: Fix a possible sleep-in-atomic bug in ql_wait_for_drvr_lock
The driver may sleep under a spinlock. The function call path is: ql_adapter_up (acquire the spinlock) ql_wait_for_drvr_lock ssleep --> may sleep To fix it, ssleep is replaced with mdelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/qlogic/qla3xxx.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c index 8ad3e24..7994d04 100644 --- a/drivers/net/ethernet/qlogic/qla3xxx.c +++ b/drivers/net/ethernet/qlogic/qla3xxx.c @@ -155,7 +155,7 @@ static int ql_wait_for_drvr_lock(struct ql3_adapter *qdev) "driver lock acquired\n"); return 1; } - ssleep(1); + mdelay(1000); } while (++i < 10); netdev_err(qdev->ndev, "Timed out waiting for driver lock...\n"); -- 1.7.9.5
[PATCH 1/2] qla3xxx: Fix a possible sleep-in-atomic bug in ql_sem_spinlock
The driver may sleep under a spinlock. The function call paths are: ql_get_full_dup (acquire the spinlock) ql_sem_spinlock ssleep --> may sleep ql_get_auto_cfg_status (acquire the spinlock) ql_sem_spinlock ssleep --> may sleep To fix it, ssleep is replaced with mdelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/qlogic/qla3xxx.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c index 9e5264d..8ad3e24 100644 --- a/drivers/net/ethernet/qlogic/qla3xxx.c +++ b/drivers/net/ethernet/qlogic/qla3xxx.c @@ -115,7 +115,7 @@ static int ql_sem_spinlock(struct ql3_adapter *qdev, value = readl(&port_regs->CommonRegs.semaphoreReg); if ((value & (sem_mask >> 16)) == sem_bits) return 0; - ssleep(1); + mdelay(1000); } while (--seconds); return -1; } -- 1.7.9.5
[BUG] atlx: a possible sleep-in-atomic bug in atl1_intr
The driver may sleep in the interrupt handler. The function call path is: atl1_intr (interrupt handler) atlx_irq_disable synchronize_irq --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[BUG] tulip/de4x5: a possible sleep-in-atomic bug in de4x5_interrupt
According to drivers/net/ethernet/dec/tulip/de4x5.c, the driver may sleep in the interrupt handler. The function call path is: de4x5_interrupt (interrupt handler) synchronize_irq --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[PATCH] macb: Fix a possible sleep-in-atomic bug in macb_tx_error_task
The driver may sleep under a spinlock. The function call path is: macb_tx_error_task (acquire the spinlock) macb_halt_tx usleep_range --> may sleep To fix it, usleep_range is replaced with udelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/cadence/macb_main.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 72a67f7..b02c806 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -645,7 +645,7 @@ static int macb_halt_tx(struct macb *bp) if (!(status & MACB_BIT(TGO))) return 0; - usleep_range(10, 250); + udelay(250); } while (time_before(halt_time, timeout)); return -ETIMEDOUT; -- 1.7.9.5
[PATCH 1/2] rtc-r7301: Fix a possible sleep-in-atomic bug in rtc7301_read_time
The driver may sleep under a spinlock. The function call path is: rtc7301_read_time (acquire the spinlock) rtc7301_wait_while_busy usleep_range --> may sleep To fix it, usleep_range is replaced with udelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/rtc/rtc-r7301.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/rtc/rtc-r7301.c b/drivers/rtc/rtc-r7301.c index 28d5408..d846e97 100644 --- a/drivers/rtc/rtc-r7301.c +++ b/drivers/rtc/rtc-r7301.c @@ -95,7 +95,7 @@ static int rtc7301_wait_while_busy(struct rtc7301_priv *priv) if (!(val & RTC7301_CONTROL_BUSY)) return 0; - usleep_range(200, 300); + udelay(300); } return -ETIMEDOUT; -- 1.7.9.5
[PATCH 2/2] rtc-r7301: Fix a possible sleep-in-atomic bug in rtc7301_set_time
The driver may sleep under a spinlock. The function call path is: rtc7301_set_time (acquire the spinlock) usleep_range --> may sleep To fix it, usleep_range is replaced with udelay. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/rtc/rtc-r7301.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/rtc/rtc-r7301.c b/drivers/rtc/rtc-r7301.c index 28d5408..a040536 100644 --- a/drivers/rtc/rtc-r7301.c +++ b/drivers/rtc/rtc-r7301.c @@ -235,7 +235,7 @@ static int rtc7301_set_time(struct device *dev, struct rtc_time *tm) spin_lock_irqsave(&priv->lock, flags); rtc7301_stop(priv); - usleep_range(200, 300); + udelay(300); rtc7301_select_bank(priv, 0); rtc7301_write_time(priv, tm, false); rtc7301_start(priv); -- 1.7.9.5
[BUG] cx88: a possible sleep-in-atomic bug in snd_cx88_switch_put
The driver may sleep under a spinlock. The function call path is: snd_cx88_switch_put (acquire the spinlock) v4l2_ctrl_find mutex_lock --> may sleep I do not find a good way to fix it, so I only report. This possible bug is found by my static analysis tool (DSAC) and checked by my code review. Thanks, Jia-Ju Bai
[PATCH] drm: Fix a possible sleep-in-atomic bug in show_leaks
The driver may sleep under a spinlock. The function call path is: drm_vma_offset_manager_destroy (acquire the spinlock) drm_mm_takedown show_leaks kmalloc(GFP_KERNEL) --> may sleep To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool(DSAC) and checked by my code review. Signed-off-by: Jia-Ju Bai --- drivers/gpu/drm/drm_mm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 61a1c8e..5b9965d 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -127,7 +127,7 @@ static void show_leaks(struct drm_mm *mm) unsigned long entries[STACKDEPTH]; char *buf; - buf = kmalloc(BUFSZ, GFP_KERNEL); + buf = kmalloc(BUFSZ, GFP_ATOMIC); if (!buf) return; -- 1.7.9.5
[PATCH] [PATCH] qla4xxx: Fix a sleep-in-atomic bug
The driver may sleep under a write spin lock, the function call path is: qla4_82xx_wr_32 (acquire the lock) qla4_82xx_crb_win_lock schedule or cpu_relax To fixed it, the lock is released before "schedule" and "cpu_relax", and the lock is acquired again after "schedule" and "cpu_relax". Signed-off-by: Jia-Ju Bai --- drivers/scsi/qla4xxx/ql4_nx.c |8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c index e91abb3..1cf5f4a 100644 --- a/drivers/scsi/qla4xxx/ql4_nx.c +++ b/drivers/scsi/qla4xxx/ql4_nx.c @@ -386,7 +386,7 @@ if (rv == 1) { write_lock_irqsave(&ha->hw_lock, flags); - qla4_82xx_crb_win_lock(ha); + qla4_82xx_crb_win_lock(ha, flags); qla4_82xx_pci_set_crbwindow_2M(ha, &off); } @@ -410,7 +410,7 @@ uint32_t qla4_82xx_rd_32(struct scsi_qla_host *ha, ulong off) if (rv == 1) { write_lock_irqsave(&ha->hw_lock, flags); - qla4_82xx_crb_win_lock(ha); + qla4_82xx_crb_win_lock(ha, flags); qla4_82xx_pci_set_crbwindow_2M(ha, &off); } data = readl((void __iomem *)off); @@ -476,7 +476,7 @@ int qla4_82xx_md_wr_32(struct scsi_qla_host *ha, uint32_t off, uint32_t data) #define CRB_WIN_LOCK_TIMEOUT 1 -int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) +int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha, unsigned long flags) { int i; int done = 0, timeout = 0; @@ -491,6 +491,7 @@ int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) timeout++; + write_unlock_irqrestore(&ha->hw_lock, flags); /* Yield CPU */ if (!in_interrupt()) schedule(); @@ -498,6 +499,7 @@ int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) for (i = 0; i < 20; i++) cpu_relax();/*This a nop instr on i386*/ } + write_lock_irqsave(&ha->hw_lock, flags); } qla4_82xx_wr_32(ha, QLA82XX_CRB_WIN_LOCK_ID, ha->func_num); return 0; -- 1.7.9.5
[PATCH] isdn: Fix a sleep-in-atomic bug
The driver may sleep under a spin lock, the function call path is: isdn_ppp_mp_receive (acquire the lock) isdn_ppp_mp_reassembly isdn_ppp_push_higher isdn_ppp_decompress isdn_ppp_ccp_reset_trans isdn_ppp_ccp_reset_alloc_state kzalloc(GFP_KERNEL) --> may sleep To fixed it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- drivers/isdn/i4l/isdn_ppp.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c index d07dd519..8aa158a 100644 --- a/drivers/isdn/i4l/isdn_ppp.c +++ b/drivers/isdn/i4l/isdn_ppp.c @@ -2364,7 +2364,7 @@ static struct ippp_ccp_reset_state *isdn_ppp_ccp_reset_alloc_state(struct ippp_s id); return NULL; } else { - rs = kzalloc(sizeof(struct ippp_ccp_reset_state), GFP_KERNEL); + rs = kzalloc(sizeof(struct ippp_ccp_reset_state), GFP_ATOMIC); if (!rs) return NULL; rs->state = CCPResetIdle; -- 1.7.9.5
[PATCH] i40e: Fix a sleep-in-atomic bug
The driver may sleep under a spin lock, and the function call path is: i40e_ndo_set_vf_port_vlan (acquire the lock by spin_lock_bh) i40e_vsi_remove_pvid i40e_vlan_stripping_disable i40e_aq_update_vsi_params i40e_asq_send_command mutex_lock --> may sleep To fixed it, the spin lock is released before "i40e_vsi_remove_pvid", and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c index 95c23fb..0fb38ca 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c +++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c @@ -3017,10 +3017,12 @@ int i40e_ndo_set_vf_port_vlan(struct net_device *netdev, int vf_id, VLAN_VID_MASK)); } + spin_unlock_bh(&vsi->mac_filter_hash_lock); if (vlan_id || qos) ret = i40e_vsi_add_pvid(vsi, vlanprio); else i40e_vsi_remove_pvid(vsi); + spin_lock_bh(&vsi->mac_filter_hash_lock); if (vlan_id) { dev_info(&pf->pdev->dev, "Setting VLAN %d, QOS 0x%x on VF %d\n", -- 1.7.9.5
[PATCH] enic: Fix a sleep-in-atomic bug
The driver may sleep under a spin lock in some function call paths. The 1st function call path is: enic_reset (acquire the lock by spin_lock) enic_stop enic_synchronize_irqs synchronize_irq --> may sleep The 2nd function call path is: enic_reset (acquire the lock by spin_lock) enic_dev_soft_reset enic_dev_wait schedule_timeout_uninterruptible --> may sleep The 3rd function call path is: enic_reset (acquire the lock by spin_lock) enic_open enic_request_intr enic_set_rx_cpu_rmap enic_free_rx_cpu_rmap free_irq_cpu_rmap --> may sleep To fix it, the "spin_lock" and "spin_unlock" are removed in enic_reset. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/cisco/enic/enic_main.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c index 4b87bee..2a9bef8 100644 --- a/drivers/net/ethernet/cisco/enic/enic_main.c +++ b/drivers/net/ethernet/cisco/enic/enic_main.c @@ -2310,7 +2310,6 @@ static void enic_reset(struct work_struct *work) rtnl_lock(); - spin_lock(&enic->enic_api_lock); enic_stop(enic->netdev); enic_dev_soft_reset(enic); enic_reset_addr_lists(enic); @@ -2318,7 +2317,6 @@ static void enic_reset(struct work_struct *work) enic_set_rss_nic_cfg(enic); enic_dev_set_ig_vlan_rewrite_mode(enic); enic_open(enic->netdev); - spin_unlock(&enic->enic_api_lock); call_netdevice_notifiers(NETDEV_REBOOT, enic->netdev); rtnl_unlock(); -- 1.7.9.5
[PATCH] enic: Fix another sleep-in-atomic bug
The driver may sleep under a spin lock in some function call paths. The 1st function call path is: enic_tx_hang_reset (acquire the lock by spin_lock) enic_stop enic_synchronize_irqs synchronize_irq --> may sleep The 2nd function call path is: enic_tx_hang_reset (acquire the lock by spin_lock) enic_dev_wait schedule_timeout_uninterruptible --> may sleep The 3rd function call path is: enic_tx_hang_reset (acquire the lock by spin_lock) enic_open enic_request_intr enic_set_rx_cpu_rmap enic_free_rx_cpu_rmap free_irq_cpu_rmap --> may sleep To fix it, the "spin_lock" and "spin_unlock" are removed in enic_tx_hang_reset. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/cisco/enic/enic_main.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c index 4b87bee..d6523e2 100644 --- a/drivers/net/ethernet/cisco/enic/enic_main.c +++ b/drivers/net/ethernet/cisco/enic/enic_main.c @@ -2330,7 +2330,6 @@ static void enic_tx_hang_reset(struct work_struct *work) rtnl_lock(); - spin_lock(&enic->enic_api_lock); enic_dev_hang_notify(enic); enic_stop(enic->netdev); enic_dev_hang_reset(enic); @@ -2339,7 +2338,6 @@ static void enic_tx_hang_reset(struct work_struct *work) enic_set_rss_nic_cfg(enic); enic_dev_set_ig_vlan_rewrite_mode(enic); enic_open(enic->netdev); - spin_unlock(&enic->enic_api_lock); call_netdevice_notifiers(NETDEV_REBOOT, enic->netdev); rtnl_unlock(); -- 1.7.9.5
[PATCH] megaraid: Fix a sleep-in-atomic bug
The driver may sleep under a spin lock, and the function call path is: mraid_mm_attach_buf (acquire the lock by spin_lock_irqsave) pci_pool_alloc(GFP_KERNEL) --> may sleep To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- drivers/scsi/megaraid/megaraid_mm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c index 4cf9ed9..c43afb8 100644 --- a/drivers/scsi/megaraid/megaraid_mm.c +++ b/drivers/scsi/megaraid/megaraid_mm.c @@ -574,7 +574,7 @@ kioc->pool_index= right_pool; kioc->free_buf = 1; - kioc->buf_vaddr = pci_pool_alloc(pool->handle, GFP_KERNEL, + kioc->buf_vaddr = pci_pool_alloc(pool->handle, GFP_ATOMIC, &kioc->buf_paddr); spin_unlock_irqrestore(&pool->lock, flags); -- 1.7.9.5
[PATCH] gadget: Fix a sleep-in-atomic bug
The driver may sleep under a spin lock, and the function call path is: ffs_epfile_io (acquire the lock by spin_lock_irq) usb_ep_alloc_request(GFP_KERNEL) --> may sleep To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- drivers/usb/gadget/function/f_fs.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index 47dda34..be90e25 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -1015,7 +1015,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) else ret = ep->status; goto error_mutex; - } else if (!(req = usb_ep_alloc_request(ep->ep, GFP_KERNEL))) { + } else if (!(req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC))) { ret = -ENOMEM; } else { req->buf = data; -- 1.7.9.5
[PATCH] iscsi: Fix a sleep-in-atomic bug
The driver may sleep under a spin lock, and the function call path is: iscsit_tpg_enable_portal_group (acquire the lock by spin_lock) iscsi_update_param_value kstrdup(GFP_KERNEL) --> may sleep To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- drivers/target/iscsi/iscsi_target_parameters.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c index fce6276..8768916 100644 --- a/drivers/target/iscsi/iscsi_target_parameters.c +++ b/drivers/target/iscsi/iscsi_target_parameters.c @@ -702,7 +702,7 @@ int iscsi_update_param_value(struct iscsi_param *param, char *value) { kfree(param->value); - param->value = kstrdup(value, GFP_KERNEL); + param->value = kstrdup(value, GFP_ATOMIC); if (!param->value) { pr_err("Unable to allocate memory for value.\n"); return -ENOMEM; -- 1.7.9.5
[PATCH V2] qla4xxx: Fix a sleep-in-atomic bug
The driver may sleep under a write spin lock, the function call path is: qla4_82xx_wr_32 (acquire the lock) qla4_82xx_crb_win_lock schedule or cpu_relax To fix it, the lock is released before "schedule" and "cpu_relax", and the lock is acquired again after "schedule" and "cpu_relax". Signed-off-by: Jia-Ju Bai --- drivers/scsi/qla4xxx/ql4_glbl.h |2 +- drivers/scsi/qla4xxx/ql4_nx.c |8 +--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/qla4xxx/ql4_glbl.h b/drivers/scsi/qla4xxx/ql4_glbl.h index bce96a5..b723bef 100644 --- a/drivers/scsi/qla4xxx/ql4_glbl.h +++ b/drivers/scsi/qla4xxx/ql4_glbl.h @@ -115,7 +115,7 @@ uint8_t qla4xxx_update_local_ifcb(struct scsi_qla_host *ha, void qla4_82xx_queue_iocb(struct scsi_qla_host *ha); void qla4_82xx_complete_iocb(struct scsi_qla_host *ha); -int qla4_82xx_crb_win_lock(struct scsi_qla_host *); +int qla4_82xx_crb_win_lock(struct scsi_qla_host *, unsigned long); void qla4_82xx_crb_win_unlock(struct scsi_qla_host *); int qla4_82xx_pci_get_crb_addr_2M(struct scsi_qla_host *, ulong *); void qla4_82xx_wr_32(struct scsi_qla_host *, ulong, u32); diff --git a/drivers/scsi/qla4xxx/ql4_nx.c b/drivers/scsi/qla4xxx/ql4_nx.c index e91abb3..1cf5f4a 100644 --- a/drivers/scsi/qla4xxx/ql4_nx.c +++ b/drivers/scsi/qla4xxx/ql4_nx.c @@ -386,7 +386,7 @@ if (rv == 1) { write_lock_irqsave(&ha->hw_lock, flags); - qla4_82xx_crb_win_lock(ha); + qla4_82xx_crb_win_lock(ha, flags); qla4_82xx_pci_set_crbwindow_2M(ha, &off); } @@ -410,7 +410,7 @@ uint32_t qla4_82xx_rd_32(struct scsi_qla_host *ha, ulong off) if (rv == 1) { write_lock_irqsave(&ha->hw_lock, flags); - qla4_82xx_crb_win_lock(ha); + qla4_82xx_crb_win_lock(ha, flags); qla4_82xx_pci_set_crbwindow_2M(ha, &off); } data = readl((void __iomem *)off); @@ -476,7 +476,7 @@ int qla4_82xx_md_wr_32(struct scsi_qla_host *ha, uint32_t off, uint32_t data) #define CRB_WIN_LOCK_TIMEOUT 1 -int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) +int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha, unsigned long flags) { int i; int done = 0, timeout = 0; @@ -491,6 +491,7 @@ int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) timeout++; + write_unlock_irqrestore(&ha->hw_lock, flags); /* Yield CPU */ if (!in_interrupt()) schedule(); @@ -498,6 +499,7 @@ int qla4_82xx_crb_win_lock(struct scsi_qla_host *ha) for (i = 0; i < 20; i++) cpu_relax();/*This a nop instr on i386*/ } + write_lock_irqsave(&ha->hw_lock, flags); } qla4_82xx_wr_32(ha, QLA82XX_CRB_WIN_LOCK_ID, ha->func_num); return 0; -- 1.7.9.5
[PATCH] mISDN: Fix a sleep-in-atomic bug
The driver may sleep under a read spin lock, and the function call path is: send_socklist (acquire the lock by read_lock) skb_copy(GFP_KERNEL) --> may sleep To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- drivers/isdn/mISDN/stack.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/isdn/mISDN/stack.c b/drivers/isdn/mISDN/stack.c index 8b7faea..422dced 100644 --- a/drivers/isdn/mISDN/stack.c +++ b/drivers/isdn/mISDN/stack.c @@ -75,7 +75,7 @@ if (sk->sk_state != MISDN_BOUND) continue; if (!cskb) - cskb = skb_copy(skb, GFP_KERNEL); + cskb = skb_copy(skb, GFP_ATOMIC); if (!cskb) { printk(KERN_WARNING "%s no skb\n", __func__); break; -- 1.7.9.5
[PATCH] bcache: Fix a sleep-in-atomic bug
The driver may sleep under a spin lock, and the function call path is: journal_wait_for_write (acquire the lock by spin_lock) closure_sync schedule --> may sleep To fix it, the lock is released before "closure_sync", and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/md/bcache/journal.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c index 1198e53..ad47c36 100644 --- a/drivers/md/bcache/journal.c +++ b/drivers/md/bcache/journal.c @@ -724,6 +724,7 @@ static struct journal_write *journal_wait_for_write(struct cache_set *c, btree_flush_write(c); } + spin_unlock(&c->journal.lock); closure_sync(&cl); spin_lock(&c->journal.lock); wait = true; -- 1.7.9.5
[PATCH] libcfs: Fix a sleep-in-atomic bug in cfs_wi_exit
The driver may sleep under a spin lock, and the function call path is: cfs_wi_exit (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..cef25c8 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -111,20 +111,20 @@ struct cfs_wi_sched { { LASSERT(!in_interrupt()); /* because we use plain spinlock */ LASSERT(!sched->ws_stopping); + LASSERT(wi->wi_running); + if (wi->wi_scheduled) { + LASSERT(!list_empty(&wi->wi_list)); + LASSERT(sched->ws_nscheduled > 0); + } + LASSERT(list_empty(&wi->wi_list)); spin_lock(&sched->ws_lock); - LASSERT(wi->wi_running); if (wi->wi_scheduled) { /* cancel pending schedules */ - LASSERT(!list_empty(&wi->wi_list)); list_del_init(&wi->wi_list); - - LASSERT(sched->ws_nscheduled > 0); sched->ws_nscheduled--; } - LASSERT(list_empty(&wi->wi_list)); - wi->wi_scheduled = 1; /* LBUG future schedule attempts */ spin_unlock(&sched->ws_lock); } -- 1.7.9.5
[PATCH] libcfs: Fix a sleep-in-atomic bug in cfs_wi_deschedule
The driver may sleep under a spin lock, and the function call path is: cfs_wi_deschedule (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..7e25eb9 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -140,6 +140,11 @@ struct cfs_wi_sched { LASSERT(!in_interrupt()); /* because we use plain spinlock */ LASSERT(!sched->ws_stopping); + if (wi->wi_scheduled) { + LASSERT(!list_empty(&wi->wi_list)); + LASSERT(sched->ws_nscheduled > 0); + } + LASSERT(list_empty(&wi->wi_list)); /* * return 0 if it's running already, otherwise return 1, which @@ -151,17 +156,11 @@ struct cfs_wi_sched { rc = !(wi->wi_running); if (wi->wi_scheduled) { /* cancel pending schedules */ - LASSERT(!list_empty(&wi->wi_list)); list_del_init(&wi->wi_list); - - LASSERT(sched->ws_nscheduled > 0); sched->ws_nscheduled--; - wi->wi_scheduled = 0; } - LASSERT(list_empty(&wi->wi_list)); - spin_unlock(&sched->ws_lock); return rc; } -- 1.7.9.5
[PATCH] libcfs: Fix a sleep-in-atomic bug in cfs_wi_schedule
The driver may sleep under a spin lock, and the function call path is: cfs_wi_schedule (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..30d28cd 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -179,12 +179,12 @@ struct cfs_wi_sched { { LASSERT(!in_interrupt()); /* because we use plain spinlock */ LASSERT(!sched->ws_stopping); + if (!wi->wi_scheduled) + LASSERT(list_empty(&wi->wi_list)); spin_lock(&sched->ws_lock); if (!wi->wi_scheduled) { - LASSERT(list_empty(&wi->wi_list)); - wi->wi_scheduled = 1; sched->ws_nscheduled++; if (!wi->wi_running) { @@ -195,8 +195,8 @@ struct cfs_wi_sched { } } - LASSERT(!list_empty(&wi->wi_list)); spin_unlock(&sched->ws_lock); + LASSERT(!list_empty(&wi->wi_list)); } EXPORT_SYMBOL(cfs_wi_schedule); -- 1.7.9.5
[PATCH V2] libcfs: Fix a sleep-in-atomic bug in cfs_wi_deschedule
The driver may sleep under a spin lock, and the function call path is: cfs_wi_deschedule (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..9c530cf 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -140,6 +140,10 @@ struct cfs_wi_sched { LASSERT(!in_interrupt()); /* because we use plain spinlock */ LASSERT(!sched->ws_stopping); + if (wi->wi_scheduled) { + LASSERT(!list_empty(&wi->wi_list)); + LASSERT(sched->ws_nscheduled > 0); + } /* * return 0 if it's running already, otherwise return 1, which @@ -151,18 +155,14 @@ struct cfs_wi_sched { rc = !(wi->wi_running); if (wi->wi_scheduled) { /* cancel pending schedules */ - LASSERT(!list_empty(&wi->wi_list)); list_del_init(&wi->wi_list); - - LASSERT(sched->ws_nscheduled > 0); sched->ws_nscheduled--; - wi->wi_scheduled = 0; } - LASSERT(list_empty(&wi->wi_list)); - spin_unlock(&sched->ws_lock); + + LASSERT(list_empty(&wi->wi_list)); return rc; } EXPORT_SYMBOL(cfs_wi_deschedule); -- 1.7.9.5
[PATCH V2] libcfs: Fix a sleep-in-atomic bug in cfs_wi_exit
The driver may sleep under a spin lock, and the function call path is: cfs_wi_exit (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..928d06d 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -111,22 +111,23 @@ struct cfs_wi_sched { { LASSERT(!in_interrupt()); /* because we use plain spinlock */ LASSERT(!sched->ws_stopping); + LASSERT(wi->wi_running); + if (wi->wi_scheduled) { + LASSERT(!list_empty(&wi->wi_list)); + LASSERT(sched->ws_nscheduled > 0); + } spin_lock(&sched->ws_lock); - LASSERT(wi->wi_running); if (wi->wi_scheduled) { /* cancel pending schedules */ - LASSERT(!list_empty(&wi->wi_list)); list_del_init(&wi->wi_list); - - LASSERT(sched->ws_nscheduled > 0); sched->ws_nscheduled--; } - LASSERT(list_empty(&wi->wi_list)); - wi->wi_scheduled = 1; /* LBUG future schedule attempts */ spin_unlock(&sched->ws_lock); + + LASSERT(list_empty(&wi->wi_list)); } EXPORT_SYMBOL(cfs_wi_exit); -- 1.7.9.5
[PATCH] libcfs: Fix a sleep-in-atomic bug in cfs_wi_sched_destroy
The driver may sleep under a spin lock, and the function call path is: cfs_wi_sched_destroy (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..e0424f6 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -302,11 +302,12 @@ static int cfs_wi_scheduler(void *arg) return; } - LASSERT(!list_empty(&sched->ws_list)); sched->ws_stopping = 1; spin_unlock(&cfs_wi_data.wi_glock); + LASSERT(!list_empty(&sched->ws_list)); + i = 2; wake_up_all(&sched->ws_waitq); -- 1.7.9.5
[PATCH] libcfs: Fix a sleep-in-atomic bug in cfs_wi_scheduler
The driver may sleep under a spin lock, and the function call path is: cfs_wi_scheduler (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/workitem.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/workitem.c b/drivers/staging/lustre/lnet/libcfs/workitem.c index dbc2a9b..9f7832e 100644 --- a/drivers/staging/lustre/lnet/libcfs/workitem.c +++ b/drivers/staging/lustre/lnet/libcfs/workitem.c @@ -212,9 +212,9 @@ static int cfs_wi_scheduler(void *arg) CWARN("Unable to bind %s on CPU partition %d\n", sched->ws_name, sched->ws_cpt); + LASSERT(sched->ws_starting == 1); spin_lock(&cfs_wi_data.wi_glock); - LASSERT(sched->ws_starting == 1); sched->ws_starting--; sched->ws_nthreads++; @@ -231,11 +231,14 @@ static int cfs_wi_scheduler(void *arg) nloops < CFS_WI_RESCHED) { wi = list_entry(sched->ws_runq.next, struct cfs_workitem, wi_list); + + spin_unlock(&sched->ws_lock); LASSERT(wi->wi_scheduled && !wi->wi_running); + LASSERT(sched->ws_nscheduled > 0); + spin_lock(&sched->ws_lock); list_del_init(&wi->wi_list); - LASSERT(sched->ws_nscheduled > 0); sched->ws_nscheduled--; wi->wi_running = 1; @@ -254,7 +257,10 @@ static int cfs_wi_scheduler(void *arg) if (list_empty(&wi->wi_list)) continue; + spin_unlock(&sched->ws_lock); LASSERT(wi->wi_scheduled); + spin_lock(&sched->ws_lock); + /* wi is rescheduled, should be on rerunq now, we * move it to runq so it can run action now */ -- 1.7.9.5
[PATCH] libcfs: Fix a sleep-in-atomic bug in cfs_percpt_lock and cfs_percpt_unlock
The driver may sleep under a spin lock, and the function call path is: cfs_percpt_lock/cfs_percpt_unlock (acquire the lock by spin_lock) LASSERT lbug_with_loc libcfs_debug_dumplog schedule and kthread_run --> may sleep To fix it, all "LASSERT" is placed out of the spin_lock and spin_unlock. Signed-off-by: Jia-Ju Bai --- drivers/staging/lustre/lnet/libcfs/libcfs_lock.c |9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/staging/lustre/lnet/libcfs/libcfs_lock.c b/drivers/staging/lustre/lnet/libcfs/libcfs_lock.c index 1967b97c..a2ce092f 100644 --- a/drivers/staging/lustre/lnet/libcfs/libcfs_lock.c +++ b/drivers/staging/lustre/lnet/libcfs/libcfs_lock.c @@ -113,9 +113,10 @@ struct cfs_percpt_lock * /* exclusive lock request */ for (i = 0; i < ncpt; i++) { + if (!i) + LASSERT(!pcl->pcl_locked); spin_lock(pcl->pcl_locks[i]); if (!i) { - LASSERT(!pcl->pcl_locked); /* nobody should take private lock after this * so I wouldn't starve for too long time */ @@ -141,11 +142,11 @@ struct cfs_percpt_lock * } for (i = ncpt - 1; i >= 0; i--) { - if (!i) { - LASSERT(pcl->pcl_locked); + if (!i) pcl->pcl_locked = 0; - } spin_unlock(pcl->pcl_locks[i]); + if (!i) + LASSERT(pcl->pcl_locked); } } EXPORT_SYMBOL(cfs_percpt_unlock); -- 1.7.9.5
Re: [PATCH] bcache: Fix a sleep-in-atomic bug
On 05/31/2017 03:23 PM, Jia-Ju Bai wrote: The driver may sleep under a spin lock, and the function call path is: journal_wait_for_write (acquire the lock by spin_lock) closure_sync schedule --> may sleep To fix it, the lock is released before "closure_sync", and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/md/bcache/journal.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/md/bcache/journal.c b/drivers/md/bcache/journal.c index 1198e53..ad47c36 100644 --- a/drivers/md/bcache/journal.c +++ b/drivers/md/bcache/journal.c @@ -724,6 +724,7 @@ static struct journal_write *journal_wait_for_write(struct cache_set *c, btree_flush_write(c); } + spin_unlock(&c->journal.lock); closure_sync(&cl); spin_lock(&c->journal.lock); wait = true; Sorry, my patch is not correct, and it will cause double unlock. Please ignore my patch. Jia-Ju Bai
[PATCH] gma500: Fix a sleep-in-atomic bug in psbfb_2d_submit
The driver may sleep under a spin lock, and the function call path is: psbfb_2d_submit (acquire the lock by spin_lock_irqsave) psb_2d_wait_available psb_spank msleep --> may sleep To fix it, the "msleep" is replaced with "mdelay" in psb_spank. Signed-off-by: Jia-Ju Bai --- drivers/gpu/drm/gma500/accel_2d.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/gma500/accel_2d.c b/drivers/gpu/drm/gma500/accel_2d.c index c51d925..7c24c8a 100644 --- a/drivers/gpu/drm/gma500/accel_2d.c +++ b/drivers/gpu/drm/gma500/accel_2d.c @@ -55,7 +55,7 @@ void psb_spank(struct drm_psb_private *dev_priv) _PSB_CS_RESET_TWOD_RESET, PSB_CR_SOFT_RESET); PSB_RSGX32(PSB_CR_SOFT_RESET); - msleep(1); + mdelay(1); PSB_WSGX32(0, PSB_CR_SOFT_RESET); wmb(); @@ -64,7 +64,7 @@ void psb_spank(struct drm_psb_private *dev_priv) wmb(); (void) PSB_RSGX32(PSB_CR_BIF_CTRL); - msleep(1); + mdelay(1); PSB_WSGX32(PSB_RSGX32(PSB_CR_BIF_CTRL) & ~_PSB_CB_CTRL_CLEAR_FAULT, PSB_CR_BIF_CTRL); (void) PSB_RSGX32(PSB_CR_BIF_CTRL); -- 1.7.9.5
[PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_hw_read_wx_2M and netxen_nic_hw_write_wx_2M
The driver may sleep under a write spin lock, and function call path is: netxen_nic_hw_read_wx_2M (acquire the lock by write_lock_irqsave) crb_win_lock netxen_pcie_sem_lock msleep --> may sleep netxen_nic_hw_write_wx_2M (acquire the lock by write_lock_irqsave) crb_win_lock netxen_pcie_sem_lock msleep --> may sleep To fix it, the "msleep" is replaced with "mdelay" in netxen_pcie_sem_lock. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c index a996801..0a9da42 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c @@ -329,7 +329,7 @@ static void __iomem *pci_base_offset(struct netxen_adapter *adapter, break; if (++timeout >= NETXEN_PCIE_SEM_TIMEOUT) return -EIO; - msleep(1); + mdelay(1); } if (id_reg) -- 1.7.9.5
[PATCH] netxen: Fix a sleep-in-atomic bug in netxen_nic_pci_mem_access_direct
The driver may sleep under a spin lock, and the function call path is: netxen_nic_pci_mem_access_direct (acquire the lock by spin_lock) ioremap --> may sleep To fix it, the lock is released before "ioremap", and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c index a996801..5ea553e 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c @@ -1419,7 +1419,9 @@ static u32 netxen_nic_io_read_2M(struct netxen_adapter *adapter, mem_base = pci_resource_start(adapter->pdev, 0) + (start & PAGE_MASK); + spin_unlock(&adapter->ahw.mem_lock); mem_ptr = ioremap(mem_base, PAGE_SIZE); + spin_lock(&adapter->ahw.mem_lock); if (mem_ptr == NULL) { ret = -EIO; goto unlock; -- 1.7.9.5
[PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed
The driver may sleep under a spin lock, and the function call path is: b43legacy_op_bss_info_changed (acquire the lock by spin_lock_irqsave) b43legacy_synchronize_irq synchronize_irq --> may sleep To fix it, the lock is released before b43legacy_synchronize_irq, and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/broadcom/b43legacy/main.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c b/drivers/net/wireless/broadcom/b43legacy/main.c index f1e3dad..31ead21 100644 --- a/drivers/net/wireless/broadcom/b43legacy/main.c +++ b/drivers/net/wireless/broadcom/b43legacy/main.c @@ -2859,7 +2859,9 @@ static void b43legacy_op_bss_info_changed(struct ieee80211_hw *hw, b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0); if (changed & BSS_CHANGED_BSSID) { + spin_unlock_irqrestore(&wl->irq_lock, flags); b43legacy_synchronize_irq(dev); + spin_lock_irqsave(&wl->irq_lock, flags); if (conf->bssid) memcpy(wl->bssid, conf->bssid, ETH_ALEN); -- 1.7.9.5
[PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_attr_interfmode_store
The driver may sleep under a spin lock, and the function call path is: b43legacy_attr_interfmode_store (acquire the lock by spin_lock_irqsave) b43legacy_radio_set_interference_mitigation b43legacy_radio_interference_mitigation_disable b43legacy_calc_nrssi_slope b43legacy_synth_pu_workaround might_sleep and msleep --> may sleep Fixing it may be complex, and a possible way is to remove spin_lock_irqsave and spin_lock_irqrestore in b43legacy_attr_interfmode_store, and the code has been protected by mutex_lock and mutex_unlock. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/broadcom/b43legacy/sysfs.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/b43legacy/sysfs.c b/drivers/net/wireless/broadcom/b43legacy/sysfs.c index 2a1da15..9ede143 100644 --- a/drivers/net/wireless/broadcom/b43legacy/sysfs.c +++ b/drivers/net/wireless/broadcom/b43legacy/sysfs.c @@ -137,14 +137,12 @@ static ssize_t b43legacy_attr_interfmode_store(struct device *dev, } mutex_lock(&wldev->wl->mutex); - spin_lock_irqsave(&wldev->wl->irq_lock, flags); err = b43legacy_radio_set_interference_mitigation(wldev, mode); if (err) b43legacyerr(wldev->wl, "Interference Mitigation not " "supported by device\n"); mmiowb(); - spin_unlock_irqrestore(&wldev->wl->irq_lock, flags); mutex_unlock(&wldev->wl->mutex); return err ? err : count; -- 1.7.9.5
[PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc
The driver may sleep in interrupt handling, and the function call path is: qed_int_sp_dpc (tasklet_init indicates it handles interrupt) qed_int_attentions qed_mcp_handle_events qed_mcp_handle_link_change qed_link_update qed_fill_link qed_mcp_get_media_type qed_ptt_acquire usleep_range --> may sleep To fix it, the "usleep_range" is replaced with "udelay". Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/qlogic/qed/qed_hw.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/qlogic/qed/qed_hw.c b/drivers/net/ethernet/qlogic/qed/qed_hw.c index a05feb3..3250cc4 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_hw.c +++ b/drivers/net/ethernet/qlogic/qed/qed_hw.c @@ -131,7 +131,7 @@ struct qed_ptt *qed_ptt_acquire(struct qed_hwfn *p_hwfn) } spin_unlock_bh(&p_hwfn->p_ptt_pool->lock); - usleep_range(1000, 2000); + udelay(1500); } DP_NOTICE(p_hwfn, "PTT acquire timeout - failed to allocate PTT\n"); -- 1.7.9.5
Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_attr_interfmode_store
On 06/01/2017 01:33 AM, Larry Finger wrote: On 05/31/2017 05:29 AM, Jia-Ju Bai wrote: The driver may sleep under a spin lock, and the function call path is: b43legacy_attr_interfmode_store (acquire the lock by spin_lock_irqsave) b43legacy_radio_set_interference_mitigation b43legacy_radio_interference_mitigation_disable b43legacy_calc_nrssi_slope b43legacy_synth_pu_workaround might_sleep and msleep --> may sleep Fixing it may be complex, and a possible way is to remove spin_lock_irqsave and spin_lock_irqrestore in b43legacy_attr_interfmode_store, and the code has been protected by mutex_lock and mutex_unlock. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/broadcom/b43legacy/sysfs.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/wireless/broadcom/b43legacy/sysfs.c b/drivers/net/wireless/broadcom/b43legacy/sysfs.c index 2a1da15..9ede143 100644 --- a/drivers/net/wireless/broadcom/b43legacy/sysfs.c +++ b/drivers/net/wireless/broadcom/b43legacy/sysfs.c @@ -137,14 +137,12 @@ static ssize_t b43legacy_attr_interfmode_store(struct device *dev, } mutex_lock(&wldev->wl->mutex); -spin_lock_irqsave(&wldev->wl->irq_lock, flags); err = b43legacy_radio_set_interference_mitigation(wldev, mode); if (err) b43legacyerr(wldev->wl, "Interference Mitigation not " "supported by device\n"); mmiowb(); -spin_unlock_irqrestore(&wldev->wl->irq_lock, flags); mutex_unlock(&wldev->wl->mutex); return err ? err : count; Jia-Ju, Did you actually observe the attempt to sleep under the spin lock, or did you discover this using some tool? In other words, have either of your patches been tested? Larry Hi, In fact, my reported bugs are found by a static analysis tool written by me, and they are checked by my review of the driver code. I admit my patches are not well tested, and they may not well fix the bugs. I am looking forward to opinions and suggestions :) Thanks, Jia-Ju Bai
Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed
On 06/01/2017 08:07 AM, Larry Finger wrote: On 05/31/2017 10:32 AM, Michael Büsch wrote: On Wed, 31 May 2017 13:26:43 +0300 Kalle Valo wrote: Jia-Ju Bai writes: The driver may sleep under a spin lock, and the function call path is: b43legacy_op_bss_info_changed (acquire the lock by spin_lock_irqsave) b43legacy_synchronize_irq synchronize_irq --> may sleep To fix it, the lock is released before b43legacy_synchronize_irq, and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/broadcom/b43legacy/main.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c b/drivers/net/wireless/broadcom/b43legacy/main.c index f1e3dad..31ead21 100644 --- a/drivers/net/wireless/broadcom/b43legacy/main.c +++ b/drivers/net/wireless/broadcom/b43legacy/main.c @@ -2859,7 +2859,9 @@ static void b43legacy_op_bss_info_changed(struct ieee80211_hw *hw, b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0); if (changed & BSS_CHANGED_BSSID) { +spin_unlock_irqrestore(&wl->irq_lock, flags); b43legacy_synchronize_irq(dev); +spin_lock_irqsave(&wl->irq_lock, flags); To me this looks like a fragile workaround and not a real fix. You can easily add new race conditions with releasing the lock like this. I think releasing the lock possibly is fine. It certainly is better than sleeping with a lock held. We disabled the device interrupts just before this line. However I think the synchronize_irq should be outside of the conditional right after the write to B43legacy_MMIO_GEN_IRQ_MASK. (So two lines above) I don't think it makes sense to only synchronize if BSS_CHANGED_BSSID is set. On the other hand b43 does not have this irq-disabling foobar anymore. So somebody must have removed it. Maybe you can find the commit that removed this stuff from b43 and port it to b43legacy? So I would vote for moving the synchronize_irq up outside of the conditional and put the unlock/lock sequence around it. And as a second patch on top of that try to remove this stuff altogether like b43 did. The patch that removed it in b43 is commit 36dbd9548e92268127b0c31b0e121e63e9207108 Author: Michael Buesch Date: Fri Sep 4 22:51:29 2009 +0200 b43: Use a threaded IRQ handler Use a threaded IRQ handler to allow locking the mutex and sleeping while executing an interrupt. This removes usage of the irq_lock spinlock, but introduces a new hardirq_lock, which is _only_ used for the PCI/SSB lowlevel hard-irq handler. Sleeping busses (SDIO) will use mutex instead. Signed-off-by: Michael Buesch Tested-by: Larry Finger Signed-off-by: John W. Linville I vaguely remember this patch. Although it is roughly a 1000-line fix, I will try to port it to b43legacy. I still have an old BCM4306 PCMCIA card that I can test in a PowerBook G4. I agree with Michael that this is the way to go. Both of Jia-Ju's patches should be rejected. Larry It is fine to me to fix the bug by porting this former patch. Thanks, Jia-Ju Bai
[PATCH] rxe: Fix a sleep-in-atomic bug in post_one_send
The driver may sleep under a spin lock, and the function call path is: post_one_send (acquire the lock by spin_lock_irqsave) init_send_wqe copy_from_user --> may sleep To fix it, the lock is released before copy_from_user, and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/infiniband/sw/rxe/rxe_verbs.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c index 83d709e..6fb7e1a 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.c +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c @@ -725,7 +725,7 @@ static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, { int num_sge = ibwr->num_sge; struct ib_sge *sge; - int i; + int i, err; u8 *p; init_send_wr(qp, &wqe->wr, ibwr); @@ -740,8 +740,11 @@ static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, sge = ibwr->sg_list; for (i = 0; i < num_sge; i++, sge++) { - if (qp->is_user && copy_from_user(p, (__user void *) - (uintptr_t)sge->addr, sge->length)) + spin_unlock_irq(&qp->sq.sq_lock); + err = copy_from_user(p, (__user void *) + (uintptr_t)sge->addr, sge->length); + spin_lock_irq(&qp->sq.sq_lock); + if (qp->is_user && err) return -EFAULT; else if (!qp->is_user) -- 1.7.9.5
[PATCH] i40iw: Add a value assignment to avoid sleep-in-atomic bug caused by uninitialized value
The value "cqp_request->waiting" indicates whether the sleeping operation should be performed, and it is not assigned in i40iw_get_cqp_request, so the driver may sleep in interrupt handling. The function call path is: i40iw_dpc (tasklet_init indicates it handles interrupt) i40iw_process_aeq i40iw_next_iw_state i40iw_hw_modify_qp (call i40iw_get_cqp_request) i40iw_handle_cqp_op i40iw_wait_event --> may sleep To fix it, "cqp_request->waiting" is assigned in "else" branch in i40iw_get_cqp_request. Signed-off-by: Jia-Ju Bai --- drivers/infiniband/hw/i40iw/i40iw_utils.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/infiniband/hw/i40iw/i40iw_utils.c b/drivers/infiniband/hw/i40iw/i40iw_utils.c index 409a378..0f4e633 100644 --- a/drivers/infiniband/hw/i40iw/i40iw_utils.c +++ b/drivers/infiniband/hw/i40iw/i40iw_utils.c @@ -326,6 +326,7 @@ struct i40iw_cqp_request *i40iw_get_cqp_request(struct i40iw_cqp *cqp, bool wait cqp_request->waiting = true; } else { atomic_set(&cqp_request->refcount, 1); + cqp_request->waiting = false; } return cqp_request; } -- 1.7.9.5
[PATCH] cw1200: Fix a sleep-in-atomic bug in cw1200_tx_confirm_cb and cw1200_cqm_bssloss_sm
The driver may sleep under a spin lock, and the function call path is: cw1200_tx_confirm_cb (acquire the lock by spin_lock) __cw1200_cqm_bssloss_sm cancel_work_sync --> may sleep cw1200_cqm_bssloss_sm __cw1200_cqm_bssloss_sm cancel_work_sync --> may sleep To fix it, the lock is released before cancel_work_sync, and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- drivers/net/wireless/st/cw1200/sta.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/net/wireless/st/cw1200/sta.c b/drivers/net/wireless/st/cw1200/sta.c index a522248..d5f7698 100644 --- a/drivers/net/wireless/st/cw1200/sta.c +++ b/drivers/net/wireless/st/cw1200/sta.c @@ -154,7 +154,9 @@ void __cw1200_cqm_bssloss_sm(struct cw1200_common *priv, int tx = 0; priv->delayed_link_loss = 0; + spin_unlock(&priv->bss_loss_lock); cancel_work_sync(&priv->bss_params_work); + spin_lock(&priv->bss_loss_lock); pr_debug("[STA] CQM BSSLOSS_SM: state: %d init %d good %d bad: %d txlock: %d uj: %d\n", priv->bss_loss_state, -- 1.7.9.5
[PATCH] cx18: Fix a sleep-in-atomic bug in snd_cx18_pcm_hw_free
The driver may sleep under a spin lock, and the function call path is: snd_cx18_pcm_hw_free (acquire the lock by spin_lock_irqsave) vfree --> may sleep To fix it, the "substream->runtime->dma_area" is passed to a temporary value, and mark it NULL when holding the lock. The memory is freed by vfree through the temporary value outside the lock holding. Signed-off-by: Jia-Ju Bai --- drivers/media/pci/cx18/cx18-alsa-pcm.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/cx18/cx18-alsa-pcm.c b/drivers/media/pci/cx18/cx18-alsa-pcm.c index 205a98d..ba83147 100644 --- a/drivers/media/pci/cx18/cx18-alsa-pcm.c +++ b/drivers/media/pci/cx18/cx18-alsa-pcm.c @@ -257,14 +257,16 @@ static int snd_cx18_pcm_hw_free(struct snd_pcm_substream *substream) { struct snd_cx18_card *cxsc = snd_pcm_substream_chip(substream); unsigned long flags; + unsigned char *dma_area; spin_lock_irqsave(&cxsc->slock, flags); if (substream->runtime->dma_area) { dprintk("freeing pcm capture region\n"); - vfree(substream->runtime->dma_area); + dma_area = substream->runtime->dma_area; substream->runtime->dma_area = NULL; } spin_unlock_irqrestore(&cxsc->slock, flags); + vfree(dma_area); return 0; } -- 1.7.9.5
[PATCH] ivtv: Fix a sleep-in-atomic bug in snd_ivtv_pcm_hw_free
The driver may sleep under a spin lock, and the function call path is: snd_ivtv_pcm_hw_free (acquire the lock by spin_lock_irqsave) vfree --> may sleep To fix it, the "substream->runtime->dma_area" is passed to a temporary value, and mark it NULL when holding the lock. The memory is freed by vfree through the temporary value outside the lock holding. Signed-off-by: Jia-Ju Bai --- drivers/media/pci/ivtv/ivtv-alsa-pcm.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c index 807ead2..92d088c 100644 --- a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c +++ b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c @@ -262,14 +262,16 @@ static int snd_ivtv_pcm_hw_free(struct snd_pcm_substream *substream) { struct snd_ivtv_card *itvsc = snd_pcm_substream_chip(substream); unsigned long flags; + unsigned char *dma_area; spin_lock_irqsave(&itvsc->slock, flags); if (substream->runtime->dma_area) { dprintk("freeing pcm capture region\n"); - vfree(substream->runtime->dma_area); + dma_area = substream->runtime->dma_area; substream->runtime->dma_area = NULL; } spin_unlock_irqrestore(&itvsc->slock, flags); + vfree(dma_area); return 0; } -- 1.7.9.5
Re: [PATCH] megaraid: Fix a sleep-in-atomic bug
On 05/31/2017 06:18 PM, Sumit Saxena wrote: -Original Message- From: Jia-Ju Bai [mailto:baijiaju1...@163.com] Sent: Wednesday, May 31, 2017 8:27 AM To: kashyap.de...@broadcom.com; sumit.sax...@broadcom.com; shivasharan.srikanteshw...@broadcom.com; j...@linux.vnet.ibm.com; martin.peter...@oracle.com Cc: megaraidlinux@broadcom.com; linux-s...@vger.kernel.org; linux- ker...@vger.kernel.org; Jia-Ju Bai Subject: [PATCH] megaraid: Fix a sleep-in-atomic bug The driver may sleep under a spin lock, and the function call path is: mraid_mm_attach_buf (acquire the lock by spin_lock_irqsave) pci_pool_alloc(GFP_KERNEL) --> may sleep To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- drivers/scsi/megaraid/megaraid_mm.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/megaraid/megaraid_mm.c b/drivers/scsi/megaraid/megaraid_mm.c index 4cf9ed9..c43afb8 100644 --- a/drivers/scsi/megaraid/megaraid_mm.c +++ b/drivers/scsi/megaraid/megaraid_mm.c @@ -574,7 +574,7 @@ kioc->pool_index = right_pool; kioc->free_buf = 1; - kioc->buf_vaddr = pci_pool_alloc(pool->handle, GFP_KERNEL, + kioc->buf_vaddr = pci_pool_alloc(pool->handle, GFP_ATOMIC, &kioc->buf_paddr); spin_unlock_irqrestore(&pool->lock, flags); This is very old driver and reached EOL. Did you face any issue because of this bug or discover this through code review? Anyways patch looks good to me. Acked-by: Sumit Saxena -- 1.7.9.5 Hi, This bug is found by a static analysis tool and my code review. Jia-Ju Bai
[PATCH] rts5208: Fix a sleep-in-atomic bug in rtsx_exclusive_enter_ss
The driver may sleep under a spin lock, and the function call path is: rtsx_exclusive_enter_ss (acquire the lock by spin_lock) rtsx_enter_ss rtsx_power_off_card sd_cleanup_work sd_stop_seq_mode sd_switch_clock sd_ddr_tuning sd_ddr_pre_tuning_tx sd_change_phase wait_timeout schedule_timeout --> may sleep To fix it, "wait_timeout" is replaced with mdelay in sd_change_phase. Signed-off-by: Jia-Ju Bai --- drivers/staging/rts5208/sd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c index bdd35b6..76bd105 100644 --- a/drivers/staging/rts5208/sd.c +++ b/drivers/staging/rts5208/sd.c @@ -1057,7 +1057,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir) rtsx_write_register(chip, SD_DCMPS_CTL, DCMPS_CHANGE, 0); rtsx_write_register(chip, SD_VP_CTL, PHASE_CHANGE, 0); - wait_timeout(10); + mdelay(10); sd_reset_dcm(chip, tune_dir); return STATUS_FAIL; } -- 1.7.9.5
[PATCH] rts5208: Fix a sleep-in-atomic bug in sd_power_off_card3v3
The driver may sleep under a spin lock, and the function call path is: rtsx_exclusive_enter_ss (acquire the lock by spin_lock) rtsx_enter_ss rtsx_power_off_card sd_power_off_card3v3 wait_timeout schedule_timeout --> may sleep To fix it, "wait_timeout" is replaced with mdelay in sd_power_off_card3v3. Signed-off-by: Jia-Ju Bai --- drivers/staging/rts5208/sd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c index bdd35b6..aa14454 100644 --- a/drivers/staging/rts5208/sd.c +++ b/drivers/staging/rts5208/sd.c @@ -5231,7 +5231,7 @@ int sd_power_off_card3v3(struct rtsx_chip *chip) return STATUS_FAIL; } - wait_timeout(50); + mdelay(50); } if (chip->asic_code) { -- 1.7.9.5
[PATCH V2] ivtv: Fix a sleep-in-atomic bug in snd_ivtv_pcm_hw_free
The driver may sleep under a spin lock, and the function call path is: snd_ivtv_pcm_hw_free (acquire the lock by spin_lock_irqsave) vfree --> may sleep To fix it, the "substream->runtime->dma_area" is passed to a temporary value, and mark it NULL when holding the lock. The memory is freed by vfree through the temporary value outside the lock holding. Signed-off-by: Jia-Ju Bai --- drivers/media/pci/ivtv/ivtv-alsa-pcm.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c index 807ead2..a692554 100644 --- a/drivers/media/pci/ivtv/ivtv-alsa-pcm.c +++ b/drivers/media/pci/ivtv/ivtv-alsa-pcm.c @@ -262,14 +262,17 @@ static int snd_ivtv_pcm_hw_free(struct snd_pcm_substream *substream) { struct snd_ivtv_card *itvsc = snd_pcm_substream_chip(substream); unsigned long flags; + unsigned char *dma_area = NULL; spin_lock_irqsave(&itvsc->slock, flags); if (substream->runtime->dma_area) { dprintk("freeing pcm capture region\n"); - vfree(substream->runtime->dma_area); + dma_area = substream->runtime->dma_area; substream->runtime->dma_area = NULL; } spin_unlock_irqrestore(&itvsc->slock, flags); + if (dma_area) + vfree(dma_area); return 0; } -- 1.7.9.5
[PATCH] cx18: Fix a sleep-in-atomic bug in snd_cx18_pcm_hw_free
The driver may sleep under a spin lock, and the function call path is: snd_cx18_pcm_hw_free (acquire the lock by spin_lock_irqsave) vfree --> may sleep To fix it, the "substream->runtime->dma_area" is passed to a temporary value, and mark it NULL when holding the lock. The memory is freed by vfree through the temporary value outside the lock holding. Signed-off-by: Jia-Ju Bai --- drivers/media/pci/cx18/cx18-alsa-pcm.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/media/pci/cx18/cx18-alsa-pcm.c b/drivers/media/pci/cx18/cx18-alsa-pcm.c index 205a98d..8c51e4c 100644 --- a/drivers/media/pci/cx18/cx18-alsa-pcm.c +++ b/drivers/media/pci/cx18/cx18-alsa-pcm.c @@ -257,14 +257,17 @@ static int snd_cx18_pcm_hw_free(struct snd_pcm_substream *substream) { struct snd_cx18_card *cxsc = snd_pcm_substream_chip(substream); unsigned long flags; + unsigned char *dma_area = NULL; spin_lock_irqsave(&cxsc->slock, flags); if (substream->runtime->dma_area) { dprintk("freeing pcm capture region\n"); - vfree(substream->runtime->dma_area); + dma_area = substream->runtime->dma_area; substream->runtime->dma_area = NULL; } spin_unlock_irqrestore(&cxsc->slock, flags); + if (dma_area) + vfree(dma_area); return 0; } -- 1.7.9.5
[PATCH V2] rxe: Fix a sleep-in-atomic bug in post_one_send
The driver may sleep under a spin lock, and the function call path is: post_one_send (acquire the lock by spin_lock_irqsave) init_send_wqe copy_from_user --> may sleep To fix it, the lock is released before copy_from_user, and the lock is acquired again after this function. The parameter "flags" is used to restore and save the irq status. Thank Leon for good advice. Signed-off-by: Jia-Ju Bai --- drivers/infiniband/sw/rxe/rxe_verbs.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c index 83d709e..7dcdf67 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.c +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c @@ -721,11 +721,11 @@ static void init_send_wr(struct rxe_qp *qp, struct rxe_send_wr *wr, static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, unsigned int mask, unsigned int length, -struct rxe_send_wqe *wqe) +struct rxe_send_wqe *wqe, unsigned long *flags) { int num_sge = ibwr->num_sge; struct ib_sge *sge; - int i; + int i, err; u8 *p; init_send_wr(qp, &wqe->wr, ibwr); @@ -742,7 +742,12 @@ static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, for (i = 0; i < num_sge; i++, sge++) { if (qp->is_user && copy_from_user(p, (__user void *) (uintptr_t)sge->addr, sge->length)) - return -EFAULT; + spin_unlock_irqrestore(&qp->sq.sq_lock, *flags); + err = copy_from_user(p, (__user void *) + (uintptr_t)sge->addr, sge->length); + spin_lock_irqsave(&qp->sq.sq_lock, *flags); + if (qp->is_user && err) + return -EFAULT; else if (!qp->is_user) memcpy(p, (void *)(uintptr_t)sge->addr, @@ -794,7 +799,7 @@ static int post_one_send(struct rxe_qp *qp, struct ib_send_wr *ibwr, send_wqe = producer_addr(sq->queue); - err = init_send_wqe(qp, ibwr, mask, length, send_wqe); + err = init_send_wqe(qp, ibwr, mask, length, send_wqe, &flags); if (unlikely(err)) goto err1; -- 1.7.9.5
[PATCH] qlcnic: Fix a sleep-in-atomic bug in qlcnic_82xx_hw_write_wx_2M and qlcnic_82xx_hw_read_wx_2M
The driver may sleep under a write spin lock, and the function call path is: qlcnic_82xx_hw_write_wx_2M (acquire the lock by write_lock_irqsave) crb_win_lock qlcnic_pcie_sem_lock usleep_range qlcnic_82xx_hw_read_wx_2M (acquire the lock by write_lock_irqsave) crb_win_lock qlcnic_pcie_sem_lock usleep_range To fix it, the usleep_range is replaced with udelay. Signed-off-by: Jia-Ju Bai --- drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c index 838cc0c..7848cf0 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_hw.c @@ -341,7 +341,7 @@ static void qlcnic_write_window_reg(u32 addr, void __iomem *bar0, u32 data) } return -EIO; } - usleep_range(1000, 1500); + udelay(1200); } if (id_reg) -- 1.7.9.5
[PATCH V3] rxe: Fix a sleep-in-atomic bug in post_one_send
The driver may sleep under a spin lock, and the function call path is: post_one_send (acquire the lock by spin_lock_irqsave) init_send_wqe copy_from_user --> may sleep To fix it, the lock is released before copy_from_user, and the lock is acquired again after this function. The parameter "flags" is used to restore and save the irq status. Thank Leon for good advice. This patch corrects the mistakes in V2. (Thank Ram for pointing it out) Signed-off-by: Jia-Ju Bai --- drivers/infiniband/sw/rxe/rxe_verbs.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c index 83d709e..5293d15 100644 --- a/drivers/infiniband/sw/rxe/rxe_verbs.c +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c @@ -721,11 +721,11 @@ static void init_send_wr(struct rxe_qp *qp, struct rxe_send_wr *wr, static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, unsigned int mask, unsigned int length, -struct rxe_send_wqe *wqe) +struct rxe_send_wqe *wqe, unsigned long *flags) { int num_sge = ibwr->num_sge; struct ib_sge *sge; - int i; + int i, err; u8 *p; init_send_wr(qp, &wqe->wr, ibwr); @@ -740,8 +740,11 @@ static int init_send_wqe(struct rxe_qp *qp, struct ib_send_wr *ibwr, sge = ibwr->sg_list; for (i = 0; i < num_sge; i++, sge++) { - if (qp->is_user && copy_from_user(p, (__user void *) - (uintptr_t)sge->addr, sge->length)) + spin_unlock_irqrestore(&qp->sq.sq_lock, *flags); + err = copy_from_user(p, (__user void *) + (uintptr_t)sge->addr, sge->length); + spin_lock_irqsave(&qp->sq.sq_lock, *flags); + if (qp->is_user && err) return -EFAULT; else if (!qp->is_user) @@ -794,7 +797,7 @@ static int post_one_send(struct rxe_qp *qp, struct ib_send_wr *ibwr, send_wqe = producer_addr(sq->queue); - err = init_send_wqe(qp, ibwr, mask, length, send_wqe); + err = init_send_wqe(qp, ibwr, mask, length, send_wqe, &flags); if (unlikely(err)) goto err1; -- 1.7.9.5
[BUG] ntb: Sleep in interrupt handling
According to ntb_transport.c, the driver may sleep in interrupt handling. The function call path is: ntb_transport_rxc_db (tasklet_init indicates it handles interrupt) ntb_process_rxc ntb_async_rx ntb_async_rx_submit schedule_timeout --> may sleep This bug is found by my static analysis tool and my code review. I hope to fix it, but I do not have a good solution. Thanks, Jia-Ju Bai
[PATCH] oss: Fix a sleep-in-atomic bug in midi_outc
The driver may sleep under a spin lock, and the function call path is: midi_outc (acquire the lock by spin_lock_irqsave) oss_broken_sleep_on schedule_timeout --> may sleep To fix it, the lock is released before oss_broken_sleep_on, and the lock is acquired again after this function. Signed-off-by: Jia-Ju Bai --- sound/oss/sequencer.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/oss/sequencer.c b/sound/oss/sequencer.c index f19da4b..3d95d752 100644 --- a/sound/oss/sequencer.c +++ b/sound/oss/sequencer.c @@ -1211,7 +1211,9 @@ static void midi_outc(int dev, unsigned char data) spin_lock_irqsave(&lock,flags); while (n && !midi_devs[dev]->outputc(dev, data)) { + spin_unlock_irqrestore(&lock, flags); oss_broken_sleep_on(&seq_sleeper, HZ/25); + spin_lock_irqsave(&lock, flags); n--; } spin_unlock_irqrestore(&lock,flags); -- 1.7.9.5
Re: [PATCH] iscsi: Fix a sleep-in-atomic bug
On 06/01/2017 02:21 PM, Nicholas A. Bellinger wrote: Hi Jia-Ju, On Wed, 2017-05-31 at 11:26 +0800, Jia-Ju Bai wrote: The driver may sleep under a spin lock, and the function call path is: iscsit_tpg_enable_portal_group (acquire the lock by spin_lock) iscsi_update_param_value kstrdup(GFP_KERNEL) --> may sleep To fix it, the "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- drivers/target/iscsi/iscsi_target_parameters.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Btw, the use of tpg->tpg_state_lock in iscsit_tpg_enable_portal_group() while checking existing state and calling iscsi_update_param_value() is not necessary, since lio_target_tpg_enable_store() is already holding iscsit_get_tpg() -> tpg->tpg_access_lock. How about the following instead to only take tpg->tpg_state_lock when updating tpg->tpg_state instead..? diff --git a/drivers/target/iscsi/iscsi_target_tpg.c b/drivers/target/iscsi/iscsi_target_tpg.c index 2e7e08d..abaabba 100644 --- a/drivers/target/iscsi/iscsi_target_tpg.c +++ b/drivers/target/iscsi/iscsi_target_tpg.c @@ -311,11 +311,9 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) struct iscsi_tiqn *tiqn = tpg->tpg_tiqn; int ret; - spin_lock(&tpg->tpg_state_lock); if (tpg->tpg_state == TPG_STATE_ACTIVE) { pr_err("iSCSI target portal group: %hu is already" " active, ignoring request.\n", tpg->tpgt); - spin_unlock(&tpg->tpg_state_lock); return -EINVAL; } /* @@ -324,10 +322,8 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) * is enforced (as per default), and remove the NONE option. */ param = iscsi_find_param_from_key(AUTHMETHOD, tpg->param_list); - if (!param) { - spin_unlock(&tpg->tpg_state_lock); + if (!param) return -EINVAL; - } if (tpg->tpg_attrib.authentication) { if (!strcmp(param->value, NONE)) { @@ -341,6 +337,7 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) goto err; } + spin_lock(&tpg->tpg_state_lock); tpg->tpg_state = TPG_STATE_ACTIVE; spin_unlock(&tpg->tpg_state_lock); @@ -353,7 +350,6 @@ int iscsit_tpg_enable_portal_group(struct iscsi_portal_group *tpg) return 0; err: - spin_unlock(&tpg->tpg_state_lock); return ret; } I think it is fine to me. Thanks, Jia-Ju Bai
Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_attr_interfmode_store
On 06/02/2017 12:11 AM, Jonathan Corbet wrote: On Thu, 01 Jun 2017 09:05:07 +0800 Jia-Ju Bai wrote: I admit my patches are not well tested, and they may not well fix the bugs. I am looking forward to opinions and suggestions :) May I politely suggest that sending out untested locking changes is a dangerous thing to do? You really should not be changing the locking in a piece of kernel code without understanding very well what the lock is protecting and being able to say why your changes are safe. Without that, the risk of introducing subtle bugs is very high. It looks like you have written a useful tool that could help us to make the kernel more robust. If you are interested in my suggestion, I would recommend that you post the sleep-in-atomic scenarios that you are finding, but refrain from "fixing" them in any case where you cannot offer a strong explanation of why your fix is correct. Thanks for working to find bugs in the kernel! jon Hi, Thanks for your good and helpful advice. I am sorry for my improper patches. I will only report bugs instead of sending improper patches when I have no good solution of fixing the bugs. Thanks, Jia-Ju Bai
[PATCH] net: caif: Fix a sleep-in-atomic bug in cfpkt_create_pfx
The kernel may sleep under a rcu read lock in cfpkt_create_pfx, and the function call path is: cfcnfg_linkup_rsp (acquire the lock by rcu_read_lock) cfctrl_linkdown_req cfpkt_create cfpkt_create_pfx alloc_skb(GFP_KERNEL) --> may sleep cfserl_receive (acquire the lock by rcu_read_lock) cfpkt_split cfpkt_create_pfx alloc_skb(GFP_KERNEL) --> may sleep There is "in_interrupt" in cfpkt_create_pfx to decide use "GFP_KERNEL" or "GFP_ATOMIC". In this situation, "GFP_KERNEL" is used because the function is called under a rcu read lock, instead in interrupt. To fix it, only "GFP_ATOMIC" is used in cfpkt_create_pfx. Signed-off-by: Jia-Ju Bai --- net/caif/cfpkt_skbuff.c |6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/net/caif/cfpkt_skbuff.c b/net/caif/cfpkt_skbuff.c index 59ce1fc..71b6ab2 100644 --- a/net/caif/cfpkt_skbuff.c +++ b/net/caif/cfpkt_skbuff.c @@ -81,11 +81,7 @@ static struct cfpkt *cfpkt_create_pfx(u16 len, u16 pfx) { struct sk_buff *skb; - if (likely(in_interrupt())) - skb = alloc_skb(len + pfx, GFP_ATOMIC); - else - skb = alloc_skb(len + pfx, GFP_KERNEL); - + skb = alloc_skb(len + pfx, GFP_ATOMIC); if (unlikely(skb == NULL)) return NULL; -- 1.7.9.5
[PATCH] net: tipc: Fix a sleep-in-atomic bug in tipc_msg_reverse
The kernel may sleep under a rcu read lock in tipc_msg_reverse, and the function call path is: tipc_l2_rcv_msg (acquire the lock by rcu_read_lock) tipc_rcv tipc_sk_rcv tipc_msg_reverse pskb_expand_head(GFP_KERNEL) --> may sleep tipc_node_broadcast tipc_node_xmit_skb tipc_node_xmit tipc_sk_rcv tipc_msg_reverse pskb_expand_head(GFP_KERNEL) --> may sleep To fix it, "GFP_KERNEL" is replaced with "GFP_ATOMIC". Signed-off-by: Jia-Ju Bai --- net/tipc/msg.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/tipc/msg.c b/net/tipc/msg.c index 312ef7d..ab30876 100644 --- a/net/tipc/msg.c +++ b/net/tipc/msg.c @@ -508,7 +508,7 @@ bool tipc_msg_reverse(u32 own_node, struct sk_buff **skb, int err) } if (skb_cloned(_skb) && - pskb_expand_head(_skb, BUF_HEADROOM, BUF_TAILROOM, GFP_KERNEL)) + pskb_expand_head(_skb, BUF_HEADROOM, BUF_TAILROOM, GFP_ATOMIC)) goto exit; /* Now reverse the concerned fields */ -- 1.7.9.5
[PATCH] rts5208: Fix a sleep-in-atomic bug in sd_send_cmd_get_rsp
The driver may sleep under a spin lock, and the function call path is: rtsx_exclusive_enter_ss (acquire the lock by spin_lock) rtsx_enter_ss rtsx_power_off_card sd_cleanup_work sd_stop_seq_mode sd_switch_clock sd_ddr_tuning sd_ddr_pre_tuning_tx sd_send_cmd_get_rsp wait_timeout schedule_timeout --> may sleep To fix it, "wait_timeout" is replaced with mdelay in sd_send_cmd_get_rsp. Signed-off-by: Jia-Ju Bai --- drivers/staging/rts5208/sd.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c index bdd35b6..fed17ff 100644 --- a/drivers/staging/rts5208/sd.c +++ b/drivers/staging/rts5208/sd.c @@ -226,7 +226,7 @@ static int sd_send_cmd_get_rsp(struct rtsx_chip *chip, u8 cmd_idx, return STATUS_FAIL; } if (rty_cnt< SD_MAX_RETRY_COUNT) { - wait_timeout(20); + mdelay(20); rty_cnt++; goto RTY_SEND_CMD; } else { -- 1.7.9.5
[PATCH] block/drbd: Fix a sleep-in-atomic bug in drbd_bcast_event
The driver may sleep under a RCU lock, and the function call path is: drbd_sync_handshake (acquire the RCU lock) drbd_asb_recover_1p drbd_khelper drbd_bcast_event genlmsg_new(GFP_NOIO) --> may sleep To fix it, GFP_NOIO is replaced with GFP_ATOMIC. This bug is found by my static analysis tool and my code review. Signed-off-by: Jia-Ju Bai --- drivers/block/drbd/drbd_nl.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c index a12f77e..713c965 100644 --- a/drivers/block/drbd/drbd_nl.c +++ b/drivers/block/drbd/drbd_nl.c @@ -4537,7 +4537,7 @@ void drbd_bcast_event(struct drbd_device *device, const struct sib_info *sib) int err = -ENOMEM; seq = atomic_inc_return(&drbd_genl_seq); - msg = genlmsg_new(NLMSG_GOODSIZE, GFP_NOIO); + msg = genlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC); if (!msg) goto failed; -- 1.7.9.5
[PATCH] block/drbd: Fix a sleep-in-atomic bug in notify_helper
The driver may sleep under a RCU lock, and the function call path is: drbd_sync_handshake (acquire the RCU lock) drbd_asb_recover_1p drbd_khelper notify_helper genlmsg_new(GFP_NOIO) --> may sleep To fix it, GFP_NOIO is replaced with GFP_ATOMIC. This bug is found by my static analysis tool and my code review. Signed-off-by: Jia-Ju Bai --- drivers/block/drbd/drbd_nl.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c index a12f77e..ad093da 100644 --- a/drivers/block/drbd/drbd_nl.c +++ b/drivers/block/drbd/drbd_nl.c @@ -4790,7 +4790,7 @@ void notify_helper(enum drbd_notification_type type, helper_info.helper_name_len = min(strlen(name), sizeof(helper_info.helper_name)); helper_info.helper_status = status; - skb = genlmsg_new(NLMSG_GOODSIZE, GFP_NOIO); + skb = genlmsg_new(NLMSG_GOODSIZE, GFP_ATOMIC); err = -ENOMEM; if (!skb) goto fail; -- 1.7.9.5
[BUG] drbd/block: A sleep-in-atomic bug in notify_helper
The driver may sleep under a RCU lock, and the function call path is: drbd_sync_handshake (acquire the RCU lock) drbd_asb_recover_1p drbd_khelper notify_helper mutex_lock --> may sleep I hope to fix it, but I do not find a proper way at present ... This bug is found by my static analysis tool and my code review. Thanks, Jia-Ju Bai
[BUG] fs/super: a possible sleep-in-atomic bug in put_super
According to fs/super.c, the kernel may sleep under a spinlock. The function call path is: put_super (acquire the spinlock) __put_super destroy_super list_lru_destroy list_lru_unregister mutex_lock --> may sleep memcg_get_cache_ids down_read --> may sleep This bug is found by my static analysis tool and my code review. Thanks, Jia-Ju Bai
[PATCH] scsi/fnic: Fix a sleep-in-atomic bug in fnic_handle_event
The driver may sleep under a spinlock, and the function call path is: fnic_handle_event (acquire the spinlock) fnic_fcoe_start_fcf_disc fcoe_ctlr_link_up mutec_lock --> may sleep To fix it, the spinlock can be released before fnic_fcoe_start_fcf_disc, and acquired again after this function. This bug is found by my static analysis tool and my code review. Signed-off-by: Jia-Ju Bai --- drivers/scsi/fnic/fnic_fcs.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c index 999fc75..4c99c96 100644 --- a/drivers/scsi/fnic/fnic_fcs.c +++ b/drivers/scsi/fnic/fnic_fcs.c @@ -265,7 +265,9 @@ void fnic_handle_event(struct work_struct *work) case FNIC_EVT_START_FCF_DISC: FNIC_FCS_DBG(KERN_DEBUG, fnic->lport->host, "Start FCF Discovery\n"); + spin_unlock_irqrestore(&fnic->fnic_lock, flags); fnic_fcoe_start_fcf_disc(fnic); + spin_lock_irqsave(&fnic->fnic_lock, flags); break; default: FNIC_FCS_DBG(KERN_DEBUG, fnic->lport->host, -- 1.7.9.5
[BUG] fs/dcache: might_sleep is called under a spinlock
According to fs/dcache.c, might_sleep is called under a spinlock, and the function call path is: d_prune_aliases (acquire the spinlock) dput might_sleep This bug is found by my static analysis tool and my code review. A possible fix is to remove might_sleep in dput. Thanks, Jia-Ju Bai
[BUG] scsi/fcoe: Sleep-in-atomic bugs in fcoe driver
According to fcoe_ctlr.c, the driver may sleep under a RCU lock, and the function call paths are: fcoe_ctlr_disc_stop_locked (acquire the RCU lock) fc_rport_logoff mutex_lock --> may sleep fcoe_ctlr_vn_disc fc_rport_login mutex_lock --> may sleep fcoe_ctlr_vn_age fc_rport_logoff mutex_lock --> may sleep These bugs are found by my static analysis tool and my code review. Thanks, Jia-Ju Bai
[BUG] scsi/libfc: Sleep-in-atomic bugs in libfc
According to fc_disc.c, the driver may sleep under a RCU lock, and the function call paths are: fc_disc_done (acquire the RCU lock) fc_rport_logoff mutex_lock --> may sleep fc_disc_done (acquire the RCU lock) fc_rport_login mutex_lock --> may sleep fc_disc_stop_rports fc_rport_logoff mutex_lock --> may sleep These bugs are found by my static analysis tool and my code review. Thanks, Jia-Ju Bai
Re: [BUG] fs/dcache: might_sleep is called under a spinlock
Thanks for your detailed explanation :) I will improve my static analysis tool. Thanks, Jia-Ju Bai On 2017/10/3 11:19, Al Viro wrote: On Tue, Oct 03, 2017 at 10:38:25AM +0800, Jia-Ju Bai wrote: According to fs/dcache.c, might_sleep is called under a spinlock, and the function call path is: d_prune_aliases (acquire the spinlock) dput might_sleep This bug is found by my static analysis tool and my code review. A possible fix is to remove might_sleep in dput. ... or to fix your static analysis tool. First of all, that call of dput() really *can* block and if we had inode->i_lock or dentry->d_lock still held at that point we'd have a real bug. However, __dentry_kill() there is called with dentry->d_inode == inode and inode->i_lock held, so dentry->d_inode is stable until inode->i_lock is dropped. Said __dentry_kill() contains if (dentry->d_inode) dentry_unlink_inode(dentry); with inode->i_lock held until that point. dentry_unlink_inode() starts with struct inode *inode = dentry->d_inode; bool hashed = !d_unhashed(dentry); if (hashed) raw_write_seqcount_begin(&dentry->d_seq); __d_clear_type_and_inode(dentry); hlist_del_init(&dentry->d_u.d_alias); if (hashed) raw_write_seqcount_end(&dentry->d_seq); spin_unlock(&dentry->d_lock); spin_unlock(&inode->i_lock); so 1) inode in there is guaranteed to be equal to the argument of d_prune_aliases() and 2) both dentry->d_lock and inode->i_lock are dropped before dentry_unlink_inode() returns. inode->i_lock is not regained in the rest of __dentry_kill(); dentry->d_lock is regained and dropped before __dentry_kill() returns. IOW, we are fine - dput() in d_prune_aliases() is called without any spinlocks held. That, BTW, is the reason for goto restart; in there, instead of just continuing the loop - if we get to that point, the list of aliases might have changed. Removing might_sleep() in dput() would've been wrong - it really might sleep when called from that point. Here's how: we used to have two links to the same file - foo/bar and baz/barf. baz/barf used to be opened, then rm -rf baz happened and later we'd called d_prune_aliases() on the inode of foo/bar. And as the loop had been executed on one CPU, on another the opened file got closed, dropping the last reference to dentry that used to be baz/barf. Note that its parent (the thing that used to be dentry of baz) is unhashed and the only contributor to its refcount is our dentry, so dput(parent) *does* drop the last remaining reference, triggering the final iput() on inode of baz, along with freeing on-disk inode, doing disk IO, etc. Again, it's not that we can't block in that dput() - it's that __dentry_kill() drops all spinlocks.
[PATCH] ext2/super: Fix a possible sleep-in-atomic bug in parse_options
The kernel may sleep under a spinlock, and the function call path is: ext2_remount parse_options match_int match_number (lib/parser.c) kmalloc(GFP_KERNEL) --> may sleep To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool and my code review. Signed-off-by: Jia-Ju Bai --- lib/parser.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/parser.c b/lib/parser.c index 3278958..bc6e2ce 100644 --- a/lib/parser.c +++ b/lib/parser.c @@ -133,7 +133,7 @@ static int match_number(substring_t *s, int *result, int base) long val; size_t len = s->to - s->from; - buf = kmalloc(len + 1, GFP_KERNEL); + buf = kmalloc(len + 1, GFP_ATOMIC); if (!buf) return -ENOMEM; memcpy(buf, s->from, len); -- 1.7.9.5
[BUG] fs/aio: A possible sleep-in-atomic bug in aio_migratepage
According to fs/aio.c, cond_resched is called under a spinlock, and the function call path is: aio_migratepage (acquire the spinlock) migrate_page_copy copy_huge_page __copy_gigantic_page cond_resched might_sleep This bug is found by my static analysis tool and my code review. A possible fix is to remove cond_resched in __copy_gigantic_page. Thanks, Jia-Ju Bai
Re: [PATCH] ext2/super: Fix a possible sleep-in-atomic bug in parse_options
Thanks for your reply. I agree that extra allocation in match_number() and match_u64int() may be unnecessary. Thanks, Jia-Ju Bai On 2017/10/7 9:37, Linus Torvalds wrote: On Fri, Oct 6, 2017 at 6:20 PM, Jia-Ju Bai wrote: To fix it, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool and my code review. I'm not saying your patch is wrong, but it's a shame that we do that extra allocation in match_number() and match_u64int(), and that we don't have anything that is just size-limited. And there really isn't anything saying that we shouldn't do the same silly thing to match_u64int(). Maybe we don't have any actual users that need it for now, but still.. Oh well. I do wonder if we shouldn't just use something like "skip leading zeroes, copy to size-limited stack location instead" because the input length really *is* limited once you skip leading zeroes (and whatever base marker we have). We might have at most a 64-bit value in octal, so 22 bytes max. But I guess just changing the two GFP_KERNEL's to GFP_ATOMIC is much simpler. Linus