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

Edward Adam Davis <[email protected]>
Newsgroups org.kernel.vger.linux-usb,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=60740c6a17a5b5eb1f5a
Tested-by: [email protected]
Signed-off-by: Edward Adam Davis <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.