The driver may sleep while holding a read lock. The function call path (from bottom to top) in Linux 4.19 is: drivers/hid/hid-core.c, 1459: hid_alloc_report_buf(GFP_KERNEL) in __hid_request ./include/linux/hid.h, 1051: __hid_request in hid_hw_request drivers/hid/hid-picolcd_leds.c, 56: hid_hw_request in picolcd_leds_set drivers/hid/hid-picolcd_leds.c, 53: _raw_spin_lock_irqsave in picolcd_leds_set drivers/hid/hid-core.c, 1459: hid_alloc_report_buf(GFP_KERNEL) in __hid_request ./include/linux/hid.h, 1051: __hid_request in hid_hw_request drivers/hid/hid-picolcd_lcd.c, 49: hid_hw_request in picolcd_set_contrast drivers/hid/hid-picolcd_lcd.c, 46: _raw_spin_lock_irqsave in picolcd_set_contrast drivers/hid/hid-core.c, 1459: hid_alloc_report_buf(GFP_KERNEL) in __hid_request ./include/linux/hid.h, 1051: __hid_request in hid_hw_request drivers/hid/hid-picolcd_core.c, 245: hid_hw_request in picolcd_reset drivers/hid/hid-picolcd_core.c, 235: _raw_spin_lock_irqsave in picolcd_reset drivers/hid/hid-core.c, 1459: hid_alloc_report_buf(GFP_KERNEL) in __hid_request ./include/linux/hid.h, 1051: __hid_request in hid_hw_request drivers/hid/hid-picolcd_core.c, 111: hid_hw_request in picolcd_send_and_wait drivers/hid/hid-picolcd_core.c, 100: _raw_spin_lock_irqsave in picolcd_send_and_wait hid_alloc_report_buf(GFP_KERNEL) can sleep at runtime. To fix these bugs, hid_hw_request() is called without holding the spinlock. These bugs are found by a static analysis tool STCheck written by myself. Signed-off-by: Jia-Ju Bai <baijiaju1990@xxxxxxxxx> --- drivers/hid/hid-picolcd_core.c | 4 ++-- drivers/hid/hid-picolcd_lcd.c | 6 ++++-- drivers/hid/hid-picolcd_leds.c | 6 ++++-- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c index 1b5c63241af0..55d1892daa15 100644 --- a/drivers/hid/hid-picolcd_core.c +++ b/drivers/hid/hid-picolcd_core.c @@ -99,8 +99,8 @@ struct picolcd_pending *picolcd_send_and_wait(struct hid_device *hdev, work = NULL; } else { data->pending = work; - hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT); spin_unlock_irqrestore(&data->lock, flags); + hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT); wait_for_completion_interruptible_timeout(&work->ready, HZ*2); spin_lock_irqsave(&data->lock, flags); data->pending = NULL; @@ -233,8 +233,8 @@ int picolcd_reset(struct hid_device *hdev) spin_unlock_irqrestore(&data->lock, flags); return -ENODEV; } - hid_hw_request(hdev, report, HID_REQ_SET_REPORT); spin_unlock_irqrestore(&data->lock, flags); + hid_hw_request(hdev, report, HID_REQ_SET_REPORT); error = picolcd_check_version(hdev); if (error) diff --git a/drivers/hid/hid-picolcd_lcd.c b/drivers/hid/hid-picolcd_lcd.c index 0c4b76de8ae5..1fd291674ffe 100644 --- a/drivers/hid/hid-picolcd_lcd.c +++ b/drivers/hid/hid-picolcd_lcd.c @@ -26,6 +26,7 @@ static int picolcd_get_contrast(struct lcd_device *ldev) static int picolcd_set_contrast(struct lcd_device *ldev, int contrast) { struct picolcd_data *data = lcd_get_data(ldev); + int status; struct hid_report *report = picolcd_out_report(REPORT_CONTRAST, data->hdev); unsigned long flags; @@ -35,9 +36,10 @@ static int picolcd_set_contrast(struct lcd_device *ldev, int contrast) data->lcd_contrast = contrast & 0x0ff; spin_lock_irqsave(&data->lock, flags); hid_set_field(report->field[0], 0, data->lcd_contrast); - if (!(data->status & PICOLCD_FAILED)) - hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT); + status = data->status; spin_unlock_irqrestore(&data->lock, flags); + if (!(status & PICOLCD_FAILED)) + hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT); return 0; } diff --git a/drivers/hid/hid-picolcd_leds.c b/drivers/hid/hid-picolcd_leds.c index 6b505a753511..6652aa6b98dd 100644 --- a/drivers/hid/hid-picolcd_leds.c +++ b/drivers/hid/hid-picolcd_leds.c @@ -32,6 +32,7 @@ void picolcd_leds_set(struct picolcd_data *data) { struct hid_report *report; + int status; unsigned long flags; if (!data->led[0]) @@ -42,9 +43,10 @@ void picolcd_leds_set(struct picolcd_data *data) spin_lock_irqsave(&data->lock, flags); hid_set_field(report->field[0], 0, data->led_state); - if (!(data->status & PICOLCD_FAILED)) - hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT); + status = data->status; spin_unlock_irqrestore(&data->lock, flags); + if (!(status & PICOLCD_FAILED)) + hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT); } static void picolcd_led_set_brightness(struct led_classdev *led_cdev, -- 2.17.1