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

Commit bc66ea45 authored by Sujeet Kumar's avatar Sujeet Kumar Committed by Mayank Rana
Browse files

USB: f_fs: Avoid using completion variable on stack 



done completion variable is local stack variable to ffs_epfile_io().
It is being used to unblock ffs_epfile_io() from USB request
completion context where done is accessed through req->context. If
ffs_epfile_io() is unblocked or interrupted due to epfile close or
any signal before USB request completion is handled, req->context is
having stale "done" reference causing invalid access. Fix this issue
by storing done completion reference with epfile structure instead of
having it on stack to have valid req->context in completion handler.

CRs-Fixed: 653761
Change-Id: I15102538d1b5bee14dfa3c7b3fa1f8e3f767cf71
Signed-off-by: default avatarSujeet Kumar <ksujeet@codeaurora.org>
Signed-off-by: default avatarMayank Rana <mrana@codeaurora.org>
parent b484e386

drivers/usb/gadget/function/f_fs.c

+18 −4
Original line number Diff line number Diff line
@@ -751,8 +751,12 @@ static const struct file_operations ffs_ep0_operations = { 


static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req)

{

	struct ffs_ep *ep = _ep->driver_data;



	ENTER();

	if (likely(req->context)) {



	/* req may be freed during unbind */

	if (ep && ep->req && likely(req->context)) {

		struct ffs_ep *ep = _ep->driver_data;

		ep->status = req->status ? req->status : req->actual;

		ffs_log("ep status %d for req %p", ep->status, req);
@@ -1061,16 +1065,24 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) 
		WARN(1, "%s: data_len == -EINVAL\n", __func__);

		ret = -EINVAL;

	} else if (!io_data->aio) {

		DECLARE_COMPLETION_ONSTACK(done);

		struct completion *done;

		bool interrupted = false;



		req = ep->req;

		req->buf      = data;

		req->length   = data_len;



		req->context  = &done;

		req->complete = ffs_epfile_io_complete;



		if (io_data->read) {

			reinit_completion(&epfile->ffs->epout_completion);

			done = &epfile->ffs->epout_completion;

		} else {

			reinit_completion(&epfile->ffs->epin_completion);

			done = &epfile->ffs->epin_completion;

		}



		req->context = done;

		ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);

		if (unlikely(ret < 0)) {

			ret = -EIO;
@@ -1079,7 +1091,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) 


		spin_unlock_irq(&epfile->ffs->eps_lock);



		if (unlikely(wait_for_completion_interruptible(&done))) {

		if (unlikely(wait_for_completion_interruptible(done))) {

			/*

			 * To avoid race condition with ffs_epfile_io_complete,

			 * dequeue the request first then check
@@ -1831,6 +1843,8 @@ static struct ffs_data *ffs_data_new(void) 
	spin_lock_init(&ffs->eps_lock);

	init_waitqueue_head(&ffs->ev.waitq);

	init_completion(&ffs->ep0req_completion);

	init_completion(&ffs->epout_completion);

	init_completion(&ffs->epin_completion);



	/* XXX REVISIT need to update it in some places, or do we? */

	ffs->ev.can_stall = 1;

drivers/usb/gadget/function/u_fs.h

+2 −1
Original line number Diff line number Diff line
@@ -175,7 +175,8 @@ struct ffs_data { 
	 */

	struct usb_request		*ep0req;		/* P: mutex */

	struct completion		ep0req_completion;	/* P: mutex */



	struct completion		epin_completion;

	struct completion		epout_completion;

	/* reference counter */

	atomic_t			ref;

	/* how many files are opened (EP0 and others) */