All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.