[PATCH] USB: gadget: f_hid: Release get report req upon unsetup

Edward Adam Davis posted 1 patch 1 month, 2 weeks ago
drivers/usb/gadget/function/f_hid.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
[PATCH] USB: gadget: f_hid: Release get report req upon unsetup
Posted by Edward Adam Davis 1 month, 2 weeks ago
The GET_REPORT request instance (hidg->get_req) is freed only within
hidg_disable() after successful binding, and only if Condition [1] is met.

Since userspace does not execute the setup, get_report_workqueue_handler()
is never invoked; consequently, Condition [1] is never met, leading to the
memory leak reported in [2].

The solution is to add memory reclamation for get_req in hidg_unbind(),
using `get_req->length` to determine whether Condition [1] applies.

Additionally, a failure in usb_gstrings_attach() also triggers the issue
reported in [2], so this is fixed at the same time.

[1]
The report is not in the list or should not be sent immediately

[2]
BUG: memory leak
unreferenced object 0xffff888112450300 (size 128):
  backtrace (crc 18381426):
    usb_ep_alloc_request+0x2e/0xd0 drivers/usb/gadget/udc/core.c:197
    hidg_bind+0x2b/0x490 drivers/usb/gadget/function/f_hid.c:1154
    usb_add_function+0xca/0x270 drivers/usb/gadget/composite.c:333
    configfs_composite_bind+0x667/0x9b0 drivers/usb/gadget/configfs.c:1802
    gadget_bind_driver+0xed/0x390 drivers/usb/gadget/udc/core.c:1662


Fixes: a139c98f760e ("USB: gadget: f_hid: Add GET_REPORT via userspace IOCTL")
Reported-by: syzbot+60740c6a17a5b5eb1f5a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=60740c6a17a5b5eb1f5a
Tested-by: syzbot+60740c6a17a5b5eb1f5a@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 drivers/usb/gadget/function/f_hid.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 3c6b43d06a6d..8d739e052e68 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -1163,8 +1163,10 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f)
 	/* maybe allocate device-global string IDs, and patch descriptors */
 	us = usb_gstrings_attach(c->cdev, ct_func_strings,
 				 ARRAY_SIZE(ct_func_string_defs));
-	if (IS_ERR(us))
-		return PTR_ERR(us);
+	if (IS_ERR(us)) {
+		status = PTR_ERR(us);
+		goto fail;
+	}
 	hidg_interface_desc.iInterface = us[CT_FUNC_HID_IDX].id;
 
 	/* allocate instance-specific interface IDs, and patch descriptors */
@@ -1585,10 +1587,18 @@ static void hidg_free(struct usb_function *f)
 static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
 {
 	struct f_hidg *hidg = func_to_hidg(f);
+	unsigned long flags;
 
 	cdev_device_del(hidg->cdev, &hidg->dev);
 	destroy_workqueue(hidg->workqueue);
 	usb_free_all_descriptors(f);
+
+	spin_lock_irqsave(&hidg->get_report_spinlock, flags);
+	if (hidg->get_req && !hidg->get_req->length) {
+		usb_ep_free_request(f->config->cdev->gadget->ep0, hidg->get_req);
+		hidg->get_req = NULL;
+	}
+	spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
 }
 
 static struct usb_function *hidg_alloc(struct usb_function_instance *fi)
-- 
2.43.0