linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: linux-input@vger.kernel.org
Cc: madcatxster@devoid-pointer.net
Subject: [bug report] HID: hid-lg4ff: Protect concurrent access to output HID report
Date: Fri, 10 Dec 2021 17:17:40 +0300	[thread overview]
Message-ID: <20211210141740.GA18820@kili> (raw)

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

                 reply	other threads:[~2021-12-10 14:18 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211210141740.GA18820@kili \
    --to=dan.carpenter@oracle.com \
    --cc=linux-input@vger.kernel.org \
    --cc=madcatxster@devoid-pointer.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).