linux-kernel - [PATCH 1/1] usb: gadget: f_hid: Conduct proper refcounting on shared f

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 17 Oct 2022 12:27:37 +0100
From:   Lee Jones <lee@...nel.org>
To:     lee@...nel.org
Cc:     linux-kernel@...r.kernel.org, Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org
Subject: [PATCH 1/1] usb: gadget: f_hid: Conduct proper refcounting on shared f_hidg pointer

A pointer to the main HID gadget struct (*f_hidg) is shared with the
character device handing (read() and write(), etc support) on open().
However external references are presently not tracked.  This could
easily lead to some unsavoury behaviour if gadget support is disabled
/ disconnected.  Keeping track of the refcount ensures that resources
are only freed *after* they are no longer in use.

Cc: Felipe Balbi <balbi@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: linux-usb@...r.kernel.org
Signed-off-by: Lee Jones <lee@...nel.org>
---
 drivers/usb/gadget/function/f_hid.c | 37 +++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index ca0a7d9eaa34e..79d4ee8a5647f 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/hid.h>
 #include <linux/idr.h>
+#include <linux/kref.h>
 #include <linux/cdev.h>
 #include <linux/mutex.h>
 #include <linux/poll.h>
@@ -77,6 +78,8 @@ struct f_hidg {
 
 	struct usb_ep			*in_ep;
 	struct usb_ep			*out_ep;
+
+	struct kref			refcount;
 };
 
 static inline struct f_hidg *func_to_hidg(struct usb_function *f)
@@ -273,6 +276,19 @@ static struct usb_gadget_strings *ct_func_strings[] = {
 	NULL,
 };
 
+static void hidg_free_resources(struct kref *ref)
+{
+	struct f_hidg *hidg = container_of(ref, struct f_hidg, refcount);
+	struct f_hid_opts *opts = container_of(hidg->func.fi, struct f_hid_opts, func_inst);
+
+	mutex_lock(&opts->lock);
+	kfree(hidg->report_desc);
+	kfree(hidg->set_report_buf);
+	kfree(hidg);
+	--opts->refcnt;
+	mutex_unlock(&opts->lock);
+}
+
 /*-------------------------------------------------------------------------*/
 /*                              Char Device                                */
 
@@ -539,7 +555,13 @@ static __poll_t f_hidg_poll(struct file *file, poll_table *wait)
 
 static int f_hidg_release(struct inode *inode, struct file *fd)
 {
+	struct f_hidg *hidg =
+		container_of(inode->i_cdev, struct f_hidg, cdev);
+
 	fd->private_data = NULL;
+
+	kref_put(&hidg->refcount, hidg_free_resources);
+
 	return 0;
 }
 
@@ -548,6 +570,8 @@ static int f_hidg_open(struct inode *inode, struct file *fd)
 	struct f_hidg *hidg =
 		container_of(inode->i_cdev, struct f_hidg, cdev);
 
+	kref_get(&hidg->refcount);
+
 	fd->private_data = hidg;
 
 	return 0;
@@ -1239,17 +1263,9 @@ static struct usb_function_instance *hidg_alloc_inst(void)
 
 static void hidg_free(struct usb_function *f)
 {
-	struct f_hidg *hidg;
-	struct f_hid_opts *opts;
+	struct f_hidg *hidg = func_to_hidg(f);
 
-	hidg = func_to_hidg(f);
-	opts = container_of(f->fi, struct f_hid_opts, func_inst);
-	kfree(hidg->report_desc);
-	kfree(hidg->set_report_buf);
-	kfree(hidg);
-	mutex_lock(&opts->lock);
-	--opts->refcnt;
-	mutex_unlock(&opts->lock);
+	kref_put(&hidg->refcount, hidg_free_resources);
 }
 
 static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
@@ -1271,6 +1287,7 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi)
 	hidg = kzalloc(sizeof(*hidg), GFP_KERNEL);
 	if (!hidg)
 		return ERR_PTR(-ENOMEM);
+	kref_init(&hidg->refcount);
 
 	opts = container_of(fi, struct f_hid_opts, func_inst);
 
-- 
2.38.0.413.g74048e4d9e-goog