Commit aeb686a9 authored by Avichal Rakesh's avatar Avichal Rakesh Committed by Greg Kroah-Hartman
Browse files

usb: gadget: uvc: Allocate uvc_requests one at a time 

Currently, the uvc gadget driver allocates all uvc_requests as one array
and deallocates them all when the video stream stops. This includes
de-allocating all the usb_requests associated with those uvc_requests.
This can lead to use-after-free issues if any of those de-allocated
usb_requests were still owned by the usb controller.

This patch is 1 of 2 patches addressing the use-after-free issue.
Instead of bulk allocating all uvc_requests as an array, this patch
allocates uvc_requests one at a time, which should allows for similar
granularity when deallocating the uvc_requests. This patch has no
functional changes other than allocating each uvc_request separately,
and similarly freeing each of them separately.

Link: https://lore.kernel.org/7cd81649-2795-45b6-8c10-b7df1055020d@google.com


Reviewed-by: default avatarDaniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: default avatarMichael Grzeschik <m.grzeschik@pengutronix.de>
Suggested-by: default avatarMichael Grzeschik <m.grzeschik@pengutronix.de>
Tested-by: default avatarMichael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: default avatarAvichal Rakesh <arakesh@google.com>
Link: https://lore.kernel.org/r/20231109004104.3467968-2-arakesh@google.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 991544dc

drivers/usb/gadget/function/uvc.h

+2 −1
Original line number Diff line number Diff line
@@ -81,6 +81,7 @@ struct uvc_request { 
	struct sg_table sgt;

	u8 header[UVCG_REQUEST_HEADER_LEN];

	struct uvc_buffer *last_buf;

	struct list_head list;

};



struct uvc_video {
@@ -102,7 +103,7 @@ struct uvc_video { 


	/* Requests */

	unsigned int req_size;

	struct uvc_request *ureq;

	struct list_head ureqs; /* all uvc_requests allocated by uvc_video */

	struct list_head req_free;

	spinlock_t req_lock;

drivers/usb/gadget/function/uvc_video.c

+49 −39
Original line number Diff line number Diff line
@@ -227,6 +227,24 @@ uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video, 
 * Request handling

 */



static void

uvc_video_free_request(struct uvc_request *ureq, struct usb_ep *ep)

{

	sg_free_table(&ureq->sgt);

	if (ureq->req && ep) {

		usb_ep_free_request(ep, ureq->req);

		ureq->req = NULL;

	}



	kfree(ureq->req_buffer);

	ureq->req_buffer = NULL;



	if (!list_empty(&ureq->list))

		list_del_init(&ureq->list);



	kfree(ureq);

}



static int uvcg_video_ep_queue(struct uvc_video *video, struct usb_request *req)

{

	int ret;
@@ -293,27 +311,12 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req) 
static int

uvc_video_free_requests(struct uvc_video *video)

{

	unsigned int i;



	if (video->ureq) {

		for (i = 0; i < video->uvc_num_requests; ++i) {

			sg_free_table(&video->ureq[i].sgt);



			if (video->ureq[i].req) {

				usb_ep_free_request(video->ep, video->ureq[i].req);

				video->ureq[i].req = NULL;

			}



			if (video->ureq[i].req_buffer) {

				kfree(video->ureq[i].req_buffer);

				video->ureq[i].req_buffer = NULL;

			}

		}

	struct uvc_request *ureq, *temp;



		kfree(video->ureq);

		video->ureq = NULL;

	}

	list_for_each_entry_safe(ureq, temp, &video->ureqs, list)

		uvc_video_free_request(ureq, video->ep);



	INIT_LIST_HEAD(&video->ureqs);

	INIT_LIST_HEAD(&video->req_free);

	video->req_size = 0;

	return 0;
@@ -322,6 +325,7 @@ uvc_video_free_requests(struct uvc_video *video) 
static int

uvc_video_alloc_requests(struct uvc_video *video)

{

	struct uvc_request *ureq;

	unsigned int req_size;

	unsigned int i;

	int ret = -ENOMEM;
@@ -332,29 +336,33 @@ uvc_video_alloc_requests(struct uvc_video *video) 
		 * max_t(unsigned int, video->ep->maxburst, 1)

		 * (video->ep->mult);



	video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);

	if (video->ureq == NULL)

		return -ENOMEM;

	for (i = 0; i < video->uvc_num_requests; i++) {

		ureq = kzalloc(sizeof(struct uvc_request), GFP_KERNEL);

		if (ureq == NULL)

			goto error;



	for (i = 0; i < video->uvc_num_requests; ++i) {

		video->ureq[i].req_buffer = kmalloc(req_size, GFP_KERNEL);

		if (video->ureq[i].req_buffer == NULL)

		INIT_LIST_HEAD(&ureq->list);



		list_add_tail(&ureq->list, &video->ureqs);



		ureq->req_buffer = kmalloc(req_size, GFP_KERNEL);

		if (ureq->req_buffer == NULL)

			goto error;



		video->ureq[i].req = usb_ep_alloc_request(video->ep, GFP_KERNEL);

		if (video->ureq[i].req == NULL)

		ureq->req = usb_ep_alloc_request(video->ep, GFP_KERNEL);

		if (ureq->req == NULL)

			goto error;



		video->ureq[i].req->buf = video->ureq[i].req_buffer;

		video->ureq[i].req->length = 0;

		video->ureq[i].req->complete = uvc_video_complete;

		video->ureq[i].req->context = &video->ureq[i];

		video->ureq[i].video = video;

		video->ureq[i].last_buf = NULL;

		ureq->req->buf = ureq->req_buffer;

		ureq->req->length = 0;

		ureq->req->complete = uvc_video_complete;

		ureq->req->context = ureq;

		ureq->video = video;

		ureq->last_buf = NULL;



		list_add_tail(&video->ureq[i].req->list, &video->req_free);

		list_add_tail(&ureq->req->list, &video->req_free);

		/* req_size/PAGE_SIZE + 1 for overruns and + 1 for header */

		sg_alloc_table(&video->ureq[i].sgt,

		sg_alloc_table(&ureq->sgt,

			       DIV_ROUND_UP(req_size - UVCG_REQUEST_HEADER_LEN,

					    PAGE_SIZE) + 2, GFP_KERNEL);

	}
@@ -489,8 +497,8 @@ static void uvcg_video_pump(struct work_struct *work) 
 */

int uvcg_video_enable(struct uvc_video *video, int enable)

{

	unsigned int i;

	int ret;

	struct uvc_request *ureq;



	if (video->ep == NULL) {

		uvcg_info(&video->uvc->func,
@@ -502,9 +510,10 @@ int uvcg_video_enable(struct uvc_video *video, int enable) 
		cancel_work_sync(&video->pump);

		uvcg_queue_cancel(&video->queue, 0);



		for (i = 0; i < video->uvc_num_requests; ++i)

			if (video->ureq && video->ureq[i].req)

				usb_ep_dequeue(video->ep, video->ureq[i].req);

		list_for_each_entry(ureq, &video->ureqs, list) {

			if (ureq->req)

				usb_ep_dequeue(video->ep, ureq->req);

		}



		uvc_video_free_requests(video);

		uvcg_queue_enable(&video->queue, 0);
@@ -536,6 +545,7 @@ int uvcg_video_enable(struct uvc_video *video, int enable) 
 */

int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)

{

	INIT_LIST_HEAD(&video->ureqs);

	INIT_LIST_HEAD(&video->req_free);

	spin_lock_init(&video->req_lock);

	INIT_WORK(&video->pump, uvcg_video_pump);