Hello HID developers,
The patch c918fe781fab: "HID: hid-lg4ff: Protect concurrent access to
output HID report" from Apr 8, 2015, leads to the following Smatch
static checker warning:
drivers/hid/hid-core.c:2142 hid_hw_request()
warn: sleeping in atomic context
drivers/hid/hid-core.c
2132 * @hdev: hid device
2133 * @report: report to send
2134 * @reqtype: hid request type
2135 */
2136 void hid_hw_request(struct hid_device *hdev,
2137 struct hid_report *report, int reqtype)
2138 {
2139 if (hdev->ll_driver->request)
2140 return hdev->ll_driver->request(hdev, report, reqtype);
^^^^^^^^^^^^^^^^^^^
There are three functions which implement ll_driver->request() in my
allmodconfig build: ishtp_hid_request(), amdtp_hid_request() and
usbhid_request(). The first two can sleep but usbhid_request() does not.
The problem is that a number of callers call hid_hw_request() with a
spinlock held. It only affect two drivers, picolcd and lg4ff. It's
possible that for those drivers we know that ->request() is going to
be usbhid_request() but I don't know the code at all.
2141
--> 2142 __hid_request(hdev, report, reqtype);
^^^^^^^^^^^^^
This function definitely sleeps.
2143 }
picolcd_send_and_wait() <- disables preempt
picolcd_reset() <- disables preempt
picolcd_operation_mode_store() <- disables preempt
picolcd_fb_send_tile() <- disables preempt
picolcd_fb_update() <- disables preempt
-> picolcd_fb_reset() <- disables preempt
picolcd_set_brightness() <- disables preempt
picolcd_leds_set() <- disables preempt
picolcd_set_contrast() <- disables preempt
lg4ff_play() <- disables preempt
lg4ff_set_autocenter_default() <- disables preempt
lg4ff_set_autocenter_ffex() <- disables preempt
lg4ff_set_range_g25() <- disables preempt
lg4ff_set_range_dfp() <- disables preempt
lg4ff_switch_compatibility_mode() <- disables preempt
Here is a sample caller:
drivers/hid/hid-picolcd_core.c
90 mutex_lock(&data->mutex);
91 spin_lock_irqsave(&data->lock, flags);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Takes a lock.
92 for (i = k = 0; i < report->maxfield; i++)
93 for (j = 0; j < report->field[i]->report_count; j++) {
94 hid_set_field(report->field[i], j, k < size ? raw_data[k] : 0);
95 k++;
96 }
97 if (data->status & PICOLCD_FAILED) {
98 kfree(work);
99 work = NULL;
100 } else {
101 data->pending = work;
102 hid_hw_request(data->hdev, report, HID_REQ_SET_REPORT);
^^^^^^^^^^^^^^^
103 spin_unlock_irqrestore(&data->lock, flags);
104 wait_for_completion_interruptible_timeout(&work->ready, HZ*2);
105 spin_lock_irqsave(&data->lock, flags);
106 data->pending = NULL;
107 }
108 spin_unlock_irqrestore(&data->lock, flags);
109 mutex_unlock(&data->mutex);
110 return work;
111 }
regards,
dan carpenter
