Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c78c87c4 authored by John Keeping's avatar John Keeping Committed by Greg Kroah-Hartman
Browse files

usb: gadget: f_hid: fix f_hidg lifetime vs cdev 



[ Upstream commit 89ff3dfac604614287ad5aad9370c3f984ea3f4b ]

The embedded struct cdev does not have its lifetime correctly tied to
the enclosing struct f_hidg, so there is a use-after-free if /dev/hidgN
is held open while the gadget is deleted.

This can readily be replicated with libusbgx's example programs (for
conciseness - operating directly via configfs is equivalent):

	gadget-hid
	exec 3<> /dev/hidg0
	gadget-vid-pid-remove
	exec 3<&-

Pull the existing device up in to struct f_hidg and make use of the
cdev_device_{add,del}() helpers.  This changes the lifetime of the
device object to match struct f_hidg, but note that it is still added
and deleted at the same time.

Fixes: 71adf118 ("USB: gadget: add HID gadget driver")
Tested-by: default avatarLee Jones <lee@kernel.org>
Reviewed-by: default avatarAndrzej Pietrasiewicz <andrzej.p@collabora.com>
Reviewed-by: default avatarLee Jones <lee@kernel.org>
Signed-off-by: default avatarJohn Keeping <john@metanate.com>
Link: https://lore.kernel.org/r/20221122123523.3068034-2-john@metanate.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
parent efa2ed93

drivers/usb/gadget/function/f_hid.c

+28 −24
Original line number Diff line number Diff line
@@ -71,7 +71,7 @@ struct f_hidg { 
	wait_queue_head_t		write_queue;

	struct usb_request		*req;



	int				minor;

	struct device			dev;

	struct cdev			cdev;

	struct usb_function		func;
@@ -84,6 +84,14 @@ static inline struct f_hidg *func_to_hidg(struct usb_function *f) 
	return container_of(f, struct f_hidg, func);

}



static void hidg_release(struct device *dev)

{

	struct f_hidg *hidg = container_of(dev, struct f_hidg, dev);



	kfree(hidg->set_report_buf);

	kfree(hidg);

}



/*-------------------------------------------------------------------------*/

/*                           Static descriptors                            */
@@ -910,9 +918,7 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) 
	struct usb_ep		*ep;

	struct f_hidg		*hidg = func_to_hidg(f);

	struct usb_string	*us;

	struct device		*device;

	int			status;

	dev_t			dev;



	/* maybe allocate device-global string IDs, and patch descriptors */

	us = usb_gstrings_attach(c->cdev, ct_func_strings,
@@ -1005,21 +1011,11 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) 


	/* create char device */

	cdev_init(&hidg->cdev, &f_hidg_fops);

	dev = MKDEV(major, hidg->minor);

	status = cdev_add(&hidg->cdev, dev, 1);

	status = cdev_device_add(&hidg->cdev, &hidg->dev);

	if (status)

		goto fail_free_descs;



	device = device_create(hidg_class, NULL, dev, NULL,

			       "%s%d", "hidg", hidg->minor);

	if (IS_ERR(device)) {

		status = PTR_ERR(device);

		goto del;

	}



	return 0;

del:

	cdev_del(&hidg->cdev);

fail_free_descs:

	usb_free_all_descriptors(f);

fail:
@@ -1250,9 +1246,7 @@ static void hidg_free(struct usb_function *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);

	put_device(&hidg->dev);

	mutex_lock(&opts->lock);

	--opts->refcnt;

	mutex_unlock(&opts->lock);
@@ -1262,8 +1256,7 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f) 
{

	struct f_hidg *hidg = func_to_hidg(f);



	device_destroy(hidg_class, MKDEV(major, hidg->minor));

	cdev_del(&hidg->cdev);

	cdev_device_del(&hidg->cdev, &hidg->dev);



	usb_free_all_descriptors(f);

}
@@ -1272,6 +1265,7 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) 
{

	struct f_hidg *hidg;

	struct f_hid_opts *opts;

	int ret;



	/* allocate and initialize one new instance */

	hidg = kzalloc(sizeof(*hidg), GFP_KERNEL);
@@ -1283,17 +1277,27 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) 
	mutex_lock(&opts->lock);

	++opts->refcnt;



	hidg->minor = opts->minor;

	device_initialize(&hidg->dev);

	hidg->dev.release = hidg_release;

	hidg->dev.class = hidg_class;

	hidg->dev.devt = MKDEV(major, opts->minor);

	ret = dev_set_name(&hidg->dev, "hidg%d", opts->minor);

	if (ret) {

		--opts->refcnt;

		mutex_unlock(&opts->lock);

		return ERR_PTR(ret);

	}



	hidg->bInterfaceSubClass = opts->subclass;

	hidg->bInterfaceProtocol = opts->protocol;

	hidg->report_length = opts->report_length;

	hidg->report_desc_length = opts->report_desc_length;

	if (opts->report_desc) {

		hidg->report_desc = kmemdup(opts->report_desc,

		hidg->report_desc = devm_kmemdup(&hidg->dev, opts->report_desc,

						 opts->report_desc_length,

						 GFP_KERNEL);

		if (!hidg->report_desc) {

			kfree(hidg);

			put_device(&hidg->dev);

			mutex_unlock(&opts->lock);

			return ERR_PTR(-ENOMEM);

		}