lkml.org 
[lkml]   [2019]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] hid: hid-picolcd: fix possible sleep-in-atomic-context bug
Date
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@gmail.com>
---
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
\
 
 \ /
  Last update: 2019-12-18 09:03    [W:0.041 / U:9.572 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site