[PATCH 1/3] usb: gadget: f_hid: fix sleep-in-atomic-context in f_hidg_get_report()

[Shunbo Shi]

The second copy_from_user() in f_hidg_get_report() is called while
holding hidg->get_report_spinlock with interrupts disabled.  If the
user page backing the ioctl buffer is not resident, copy_from_user()
takes a page fault and sleeps, which is not allowed with a spinlock
held: it triggers might_sleep() warnings with
CONFIG_DEBUG_ATOMIC_SLEEP and can deadlock the scheduler on other
CPUs spinning on the lock.

The first copy_from_user() into the freshly allocated entry is
already done before the spinlock is taken; only the "report already
exists, update it" branch still copies inside the critical section.

Since the data has already been fully copied into entry->report_data
before taking the lock, updating an existing report only needs a
plain assignment between two kernel-memory objects, which is safe
inside the spinlock.

Fixes: a139c98f760e ("USB: gadget: f_hid: Add GET_REPORT via userspace IOCTL")
Signed-off-by: Shunbo Shi <shishunbo@xxxxxxxxxxxxxxxxxxxx>
---
 drivers/usb/gadget/function/f_hid.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 3c6b43d06a6d..b778eb633b79 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -654,6 +654,11 @@ static int f_hidg_get_report(struct file *file, struct usb_hidg_report __user *b
 	if (!entry)
 		return -ENOMEM;
 
+	/*
+	 * Copy the report from userspace into the entry before taking the
+	 * spinlock: copy_from_user() may sleep, so it must not be called
+	 * with the spinlock held (and interrupts disabled).
+	 */
 	if (copy_from_user(&entry->report_data, buffer,
 				sizeof(struct usb_hidg_report))) {
 		ERROR(cdev, "copy_from_user error\n");
@@ -668,13 +673,7 @@ static int f_hidg_get_report(struct file *file, struct usb_hidg_report __user *b
 
 	if (ptr) {
 		/* Report already exists in list - update it */
-		if (copy_from_user(&ptr->report_data, buffer,
-				sizeof(struct usb_hidg_report))) {
-			spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
-			ERROR(cdev, "copy_from_user error\n");
-			kfree(entry);
-			return -EINVAL;
-		}
+		ptr->report_data = entry->report_data;
 		kfree(entry);
 	} else {
 		/* Report does not exist in list - add it */
-- 
2.53.0