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

Commit de0231b9 authored by Jack Pham's avatar Jack Pham Committed by Matt Wagantall
Browse files

usb: gadget: diag: Add reference counting to diag_context 



The diag_context structure is allocated and freed each time the
function is bound and unbound. However, since the diag client is
only aware of connect/disconnect status and not whether the function
is bound, there could be a race in which a late usb_diag_read() or
write() is executing when unbind happens and frees the structure.

To prevent this, add a kref object to the diag_context structure
and make sure it is incremented appropriately during the entry to
usb_diag_read/write(). We'll use the kref_put_spinlock_irqsave()
variant to ensure that the lock (which is also part of diag_context)
is taken before executing the final put().

Change-Id: I732a65445f067e1899423397bc33a48a8fb7224d
Signed-off-by: default avatarJack Pham <jackp@codeaurora.org>
parent 655a39d4

drivers/usb/gadget/function/f_diag.c

+43 −4
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#include <linux/init.h>

#include <linux/module.h>

#include <linux/kernel.h>

#include <linux/kref.h>

#include <linux/platform_device.h>

#include <linux/ratelimit.h>
@@ -155,6 +156,7 @@ struct diag_context { 
	struct usb_composite_dev *cdev;

	int (*update_pid_and_serial_num)(uint32_t, const char *);

	struct usb_diag_ch *ch;

	struct kref kref;



	/* pkt counters */

	unsigned long dpkts_tolaptop;
@@ -172,6 +174,16 @@ static inline struct diag_context *func_to_diag(struct usb_function *f) 
	return container_of(f, struct diag_context, function);

}



/* Called with ctxt->lock held; i.e. only use with kref_put_spinlock_irqsave */

static void diag_context_release(struct kref *kref)

{

	struct diag_context *ctxt =

		container_of(kref, struct diag_context, kref);



	spin_unlock(&ctxt->lock);

	kfree(ctxt);

}



static void diag_update_pid_and_serial_num(struct diag_context *ctxt)

{

	struct usb_composite_dev *cdev = ctxt->cdev;
@@ -241,6 +253,9 @@ static void diag_write_complete(struct usb_ep *ep, 


	if (ctxt->ch && ctxt->ch->notify)

		ctxt->ch->notify(ctxt->ch->priv, USB_DIAG_WRITE_DONE, d_req);



	kref_put_spinlock_irqsave(&ctxt->kref, diag_context_release,

			&ctxt->lock);

}



static void diag_read_complete(struct usb_ep *ep,
@@ -261,6 +276,9 @@ static void diag_read_complete(struct usb_ep *ep, 


	if (ctxt->ch && ctxt->ch->notify)

		ctxt->ch->notify(ctxt->ch->priv, USB_DIAG_READ_DONE, d_req);



	kref_put_spinlock_irqsave(&ctxt->kref, diag_context_release,

			&ctxt->lock);

}



/**
@@ -445,6 +463,7 @@ int usb_diag_read(struct usb_diag_ch *ch, struct diag_request *d_req) 


	req = list_first_entry(&ctxt->read_pool, struct usb_request, list);

	list_del(&req->list);

	kref_get(&ctxt->kref); /* put called in complete callback */

	spin_unlock_irqrestore(&ctxt->lock, flags);



	req->buf = d_req->buf;
@@ -454,6 +473,8 @@ int usb_diag_read(struct usb_diag_ch *ch, struct diag_request *d_req) 
	/* make sure context is still valid after releasing lock */

	if (ctxt != ch->priv_usb) {

		usb_ep_free_request(out, req);

		kref_put_spinlock_irqsave(&ctxt->kref, diag_context_release,

				&ctxt->lock);

		return -EIO;

	}
@@ -461,11 +482,16 @@ int usb_diag_read(struct usb_diag_ch *ch, struct diag_request *d_req) 
		/* If error add the link to linked list again*/

		spin_lock_irqsave(&ctxt->lock, flags);

		list_add_tail(&req->list, &ctxt->read_pool);

		spin_unlock_irqrestore(&ctxt->lock, flags);

		/* 1 error message for every 10 sec */

		if (__ratelimit(&rl))

			ERROR(ctxt->cdev, "%s: cannot queue"

				" read request\n", __func__);



		if (kref_put(&ctxt->kref, diag_context_release))

			/* diag_context_release called spin_unlock already */

			local_irq_restore(flags);

		else

			spin_unlock_irqrestore(&ctxt->lock, flags);

		return -EIO;

	}
@@ -514,6 +540,7 @@ int usb_diag_write(struct usb_diag_ch *ch, struct diag_request *d_req) 


	req = list_first_entry(&ctxt->write_pool, struct usb_request, list);

	list_del(&req->list);

	kref_get(&ctxt->kref); /* put called in complete callback */

	spin_unlock_irqrestore(&ctxt->lock, flags);



	req->buf = d_req->buf;
@@ -523,6 +550,8 @@ int usb_diag_write(struct usb_diag_ch *ch, struct diag_request *d_req) 
	/* make sure context is still valid after releasing lock */

	if (ctxt != ch->priv_usb) {

		usb_ep_free_request(in, req);

		kref_put_spinlock_irqsave(&ctxt->kref, diag_context_release,

				&ctxt->lock);

		return -EIO;

	}
@@ -534,6 +563,11 @@ int usb_diag_write(struct usb_diag_ch *ch, struct diag_request *d_req) 
		if (__ratelimit(&rl))

			ERROR(ctxt->cdev, "%s: cannot queue"

				" read request\n", __func__);



		if (kref_put(&ctxt->kref, diag_context_release))

			/* diag_context_release called spin_unlock already */

			local_irq_restore(flags);

		else

			spin_unlock_irqrestore(&ctxt->lock, flags);

		return -EIO;

	}
@@ -646,8 +680,12 @@ static void diag_function_unbind(struct usb_configuration *c, 
	/* Free any pending USB requests from last session */

	spin_lock_irqsave(&ctxt->lock, flags);

	free_reqs(ctxt);



	if (kref_put(&ctxt->kref, diag_context_release))

		/* diag_context_release called spin_unlock already */

		local_irq_restore(flags);

	else

		spin_unlock_irqrestore(&ctxt->lock, flags);

	kfree(ctxt);

}



static int diag_function_bind(struct usb_configuration *c,
@@ -761,6 +799,7 @@ int diag_function_add(struct usb_configuration *c, const char *name, 
	dev->function.unbind = diag_function_unbind;

	dev->function.set_alt = diag_function_set_alt;

	dev->function.disable = diag_function_disable;

	kref_init(&dev->kref);

	spin_lock_init(&dev->lock);

	INIT_LIST_HEAD(&dev->read_pool);

	INIT_LIST_HEAD(&dev->write_pool);