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

Commit 825084cd authored by Krzysztof Opasiak's avatar Krzysztof Opasiak Committed by Gerrit - the friendly Code Review server
Browse files

usb: gadget: f_hid: fix: Move IN request allocation to set_alt() 



commit 749494b6bdbbaf0899aa1c62a1ad74cd747bce47 upstream.

Since commit: ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()")
we cannot allocate any requests in bind() as we check if we should
align request buffer based on endpoint descriptor which is assigned
in set_alt().

Allocating request in bind() function causes a NULL pointer
dereference.

This commit moves allocation of IN request from bind() to set_alt()
to prevent this issue.

Change-Id: I02a832bfd3008efb041450156da11e62f1ec8e5c
Fixes: ba1582f22231 ("usb: gadget: f_hid: use alloc_ep_req()")
Cc: stable@vger.kernel.org
Tested-by: default avatarDavid Lechner <david@lechnology.com>
Signed-off-by: default avatarKrzysztof Opasiak <k.opasiak@samsung.com>
Signed-off-by: default avatarFelipe Balbi <felipe.balbi@linux.intel.com>
Cc: Bin Liu <b-liu@ti.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Git-commit: 8dd5c0c47332c67a04830e3842d4d6282f1824fd
Git-repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git


Signed-off-by: default avatarAjay Agarwal <ajaya@codeaurora.org>
parent 8e9ed04d

drivers/usb/gadget/function/f_hid.c

+67 −22
Original line number Diff line number Diff line
@@ -284,6 +284,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, 
			    size_t count, loff_t *offp)

{

	struct f_hidg *hidg  = file->private_data;

	struct usb_request *req;

	unsigned long flags;

	ssize_t status = -ENOMEM;
@@ -293,7 +294,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, 
	spin_lock_irqsave(&hidg->write_spinlock, flags);



#define WRITE_COND (!hidg->write_pending)



try_again:

	/* write queue */

	while (!WRITE_COND) {

		spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -308,6 +309,7 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, 
	}



	hidg->write_pending = 1;

	req = hidg->req;

	count  = min_t(unsigned, count, hidg->report_length);



	spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -320,24 +322,38 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, 
		goto release_write_pending;

	}



	hidg->req->status   = 0;

	hidg->req->zero     = 0;

	hidg->req->length   = count;

	hidg->req->complete = f_hidg_req_complete;

	hidg->req->context  = hidg;

	spin_lock_irqsave(&hidg->write_spinlock, flags);



	/* we our function has been disabled by host */

	if (!hidg->req) {

		free_ep_req(hidg->in_ep, hidg->req);

		/*

		 * TODO

		 * Should we fail with error here?

		 */

		goto try_again;

	}



	req->status   = 0;

	req->zero     = 0;

	req->length   = count;

	req->complete = f_hidg_req_complete;

	req->context  = hidg;



	status = usb_ep_queue(hidg->in_ep, hidg->req, GFP_ATOMIC);

	if (status < 0) {

		ERROR(hidg->func.config->cdev,

			"usb_ep_queue error on int endpoint %zd\n", status);

		goto release_write_pending;

		goto release_write_pending_unlocked;

	} else {

		status = count;

	}

	spin_unlock_irqrestore(&hidg->write_spinlock, flags);



	return status;

release_write_pending:

	spin_lock_irqsave(&hidg->write_spinlock, flags);

release_write_pending_unlocked:

	hidg->write_pending = 0;

	spin_unlock_irqrestore(&hidg->write_spinlock, flags);
@@ -541,12 +557,23 @@ static void hidg_disable(struct usb_function *f) 
		kfree(list);

	}

	spin_unlock_irqrestore(&hidg->read_spinlock, flags);



	spin_lock_irqsave(&hidg->write_spinlock, flags);

	if (!hidg->write_pending) {

		free_ep_req(hidg->in_ep, hidg->req);

		hidg->write_pending = 1;

	}



	hidg->req = NULL;

	spin_unlock_irqrestore(&hidg->write_spinlock, flags);

}



static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)

{

	struct usb_composite_dev		*cdev = f->config->cdev;

	struct f_hidg				*hidg = func_to_hidg(f);

	struct usb_request			*req_in = NULL;

	unsigned long				flags;

	int i, status = 0;



	VDBG(cdev, "hidg_set_alt intf:%d alt:%d\n", intf, alt);
@@ -567,6 +594,12 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) 
			goto fail;

		}

		hidg->in_ep->driver_data = hidg;



		req_in = hidg_alloc_ep_req(hidg->in_ep, hidg->report_length);

		if (!req_in) {

			status = -ENOMEM;

			goto disable_ep_in;

		}

	}
@@ -578,12 +611,12 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) 
					    hidg->out_ep);

		if (status) {

			ERROR(cdev, "config_ep_by_speed FAILED!\n");

			goto fail;

			goto free_req_in;

		}

		status = usb_ep_enable(hidg->out_ep);

		if (status < 0) {

			ERROR(cdev, "Enable OUT endpoint FAILED!\n");

			goto fail;

			goto free_req_in;

		}

		hidg->out_ep->driver_data = hidg;
@@ -599,17 +632,37 @@ static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) 
				req->context  = hidg;

				status = usb_ep_queue(hidg->out_ep, req,

						      GFP_ATOMIC);

				if (status)

				if (status) {

					ERROR(cdev, "%s queue req --> %d\n",

						hidg->out_ep->name, status);

					free_ep_req(hidg->out_ep, req);

				}

			} else {

				usb_ep_disable(hidg->out_ep);

				status = -ENOMEM;

				goto fail;

				goto disable_out_ep;

			}

		}

	}



	if (hidg->in_ep != NULL) {

		spin_lock_irqsave(&hidg->write_spinlock, flags);

		hidg->req = req_in;

		hidg->write_pending = 0;

		spin_unlock_irqrestore(&hidg->write_spinlock, flags);



		wake_up(&hidg->write_queue);

	}

	return 0;

disable_out_ep:

	usb_ep_disable(hidg->out_ep);

free_req_in:

	if (req_in)

		free_ep_req(hidg->in_ep, req_in);



disable_ep_in:

	if (hidg->in_ep)

		usb_ep_disable(hidg->in_ep);



fail:

	return status;

}
@@ -658,12 +711,6 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) 
		goto fail;

	hidg->out_ep = ep;



	/* preallocate request and buffer */

	status = -ENOMEM;

	hidg->req = alloc_ep_req(hidg->in_ep, hidg->report_length);

	if (!hidg->req)

		goto fail;



	/* set descriptor dynamic values */

	hidg_interface_desc.bInterfaceSubClass = hidg->bInterfaceSubClass;

	hidg_interface_desc.bInterfaceProtocol = hidg->bInterfaceProtocol;
@@ -690,6 +737,8 @@ static int hidg_bind(struct usb_configuration *c, struct usb_function *f) 
		goto fail;



	spin_lock_init(&hidg->write_spinlock);

	hidg->write_pending = 1;

	hidg->req = NULL;

	spin_lock_init(&hidg->read_spinlock);

	init_waitqueue_head(&hidg->write_queue);

	init_waitqueue_head(&hidg->read_queue);
@@ -954,10 +1003,6 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f) 
	device_destroy(hidg_class, MKDEV(major, hidg->minor));

	cdev_del(&hidg->cdev);



	/* disable/free request and end point */

	usb_ep_disable(hidg->in_ep);

	free_ep_req(hidg->in_ep, hidg->req);



	usb_free_all_descriptors(f);

}