[PATCH] usb: gadget: f_hid: keep GET_REPORT request across disable

[PATCH] usb: gadget: f_hid: keep GET_REPORT request across disable

[Keisuke Tsukuda]

hidg_disable() frees hidg->get_req when a GET_REPORT is waiting for
userspace. A USB reset disables and re-enables the existing function
without calling hidg_bind(), so the request remains NULL. Every later
GET_REPORT then times out until the gadget is unbound and bound again.

Keep the request allocated for the lifetime of the bound function and
free it from hidg_unbind(). Cancel and wake a pending operation during
disable, and use a request generation counter to stop work from an old
configuration from replying to a newer control request.

A pending get_req leak fix also adds cleanup at unbind, but it leaves the
early free in hidg_disable() and therefore does not address this reset
failure.

The failure was reproduced on usb-testing with dummy_hcd: the interrupted
request failed with ESHUTDOWN and subsequent requests timed out. It was
also reproduced with a Linux xHCI host issuing a USB reset to a
Raspberry Pi 4 using DWC2. The change passed eight reset timings under
KASAN and the same physical Linux-host test.

Fixes: a139c98f760e ("USB: gadget: f_hid: Add GET_REPORT via userspace IOCTL")
Link: https://lore.kernel.org/all/tencent_F5AB9201DDA25D8E1925CF818D87258B8D07@qq.com/
Cc: stable@vger.kernel.org
Signed-off-by: Keisuke Tsukuda <tkdkei@outlook.jp>
---
 drivers/usb/gadget/function/f_hid.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 3c6b43d06a..99d9666172 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -96,6 +96,9 @@ struct f_hidg {
 	struct usb_request		*get_req;
 	struct usb_hidg_report		get_report;
 	bool				get_report_returned;
+	/* Cancel pending work and distinguish requests across reconfiguration. */
+	bool				get_report_cancelled;
+	unsigned int			get_report_req_tag;
 	int				get_report_req_report_id;
 	int				get_report_req_report_length;
 	spinlock_t			get_report_spinlock;
@@ -562,10 +565,16 @@ static void get_report_workqueue_handler(struct work_struct *work)
 	struct usb_request		*req;
 	struct report_entry *ptr;
 	unsigned long	flags;
+	unsigned int	req_tag;
 
 	int status = 0;
 
 	spin_lock_irqsave(&hidg->get_report_spinlock, flags);
+	if (hidg->get_report_cancelled) {
+		spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
+		return;
+	}
+	req_tag = hidg->get_report_req_tag;
 	req = hidg->get_req;
 	if (!req) {
 		spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
@@ -598,6 +607,11 @@ static void get_report_workqueue_handler(struct work_struct *work)
 		status = wait_event_interruptible_timeout(hidg->get_queue, !GET_REPORT_COND,
 					msecs_to_jiffies(GET_REPORT_TIMEOUT_MS));
 		spin_lock_irqsave(&hidg->get_report_spinlock, flags);
+		if (hidg->get_report_cancelled ||
+		    req_tag != hidg->get_report_req_tag) {
+			spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
+			return;
+		}
 		req = hidg->get_req;
 		if (!req) {
 			spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
@@ -864,6 +878,8 @@ static int hidg_setup(struct usb_function *f,
 		 * GET_REPORT the request was actually for.
 		 */
 		spin_lock_irqsave(&hidg->get_report_spinlock, flags);
+		hidg->get_report_cancelled = false;
+		hidg->get_report_req_tag++;
 		hidg->get_report_req_report_id = value & 0xff;
 		hidg->get_report_req_report_length = length;
 		spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
@@ -997,12 +1013,11 @@ static void hidg_disable(struct usb_function *f)
 	}
 
 	spin_lock_irqsave(&hidg->get_report_spinlock, flags);
-	if (!hidg->get_report_returned) {
-		usb_ep_free_request(f->config->cdev->gadget->ep0, hidg->get_req);
-		hidg->get_req = NULL;
-		hidg->get_report_returned = true;
-	}
+	hidg->get_report_cancelled = true;
+	hidg->get_report_req_tag++;
+	hidg->get_report_returned = true;
 	spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
+	wake_up(&hidg->get_queue);
 
 	spin_lock_irqsave(&hidg->read_spinlock, flags);
 	hidg->disabled = true;
@@ -1588,6 +1603,7 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
 
 	cdev_device_del(hidg->cdev, &hidg->dev);
 	destroy_workqueue(hidg->workqueue);
+	usb_ep_free_request(c->cdev->gadget->ep0, hidg->get_req);
 	usb_free_all_descriptors(f);
 }
 
-- 
2.50.1 (Apple Git-155)