Skip to contentSkip to conversation

FAILED: patch "[PATCH] usb: gadget: f_hid: fix: Prevent accessing released memory" failed to apply to 4.4-stable tree

PATCH [email protected] 1 participant 1 message Mar 10, 2017

Subsystems (2)
Stable branch USB
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From aa65d11aa008f4de58a9cee7e121666d9d68505e Mon Sep 17 00:00:00 2001

From: Krzysztof Opasiak <[email protected]>
Date: Thu, 19 Jan 2017 18:55:28 +0100
Subject: [PATCH] usb: gadget: f_hid: fix: Prevent accessing released memory

When we unlock our spinlock to copy data to user we may get disabled by USB host and free the whole list of completed out requests including the one from which we are copying the data to user memory.

To prevent from this let's remove our working element from the list and place it back only if there is sth left when we finish with it.

Fixes 99c515005857 ("usb: gadget: hidg: register OUT INT endpoint for SET_REPORT")
Cc [email protected]
Tested-by David Lechner <[email protected]>
Signed-off-by Krzysztof Opasiak <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
drivers/usb/gadget/function/f_hid.c +20 -4
index 62cf79815506..4ab74204efe2 100644
@@ -277,6 +277,13 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer,
 	/* pick the first one */
 	list = list_first_entry(&hidg->completed_out_req,
 				struct f_hidg_req_list, list);
+
+	/*
+	 * Remove this from list to protect it from beign free()
+	 * while host disables our function
+	 */
+	list_del(&list->list);
+
 	req = list->req;
 	count = min_t(unsigned int, count, req->actual - list->pos);
 	spin_unlock_irqrestore(&hidg->spinlock, flags);

@@ -292,15 +299,20 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer,
 	 * call, taking into account its current read position.
 	 */
 	if (list->pos == req->actual) {
-		spin_lock_irqsave(&hidg->spinlock, flags);
-		list_del(&list->list);
 		kfree(list);
-		spin_unlock_irqrestore(&hidg->spinlock, flags);
 
 		req->length = hidg->report_length;
 		ret = usb_ep_queue(hidg->out_ep, req, GFP_KERNEL);
-		if (ret < 0)
+		if (ret < 0) {
+			free_ep_req(hidg->out_ep, req);
 			return ret;
+		}
+	} else {
+		spin_lock_irqsave(&hidg->spinlock, flags);
+		list_add(&list->list, &hidg->completed_out_req);
+		spin_unlock_irqrestore(&hidg->spinlock, flags);
+
+		wake_up(&hidg->read_queue);
 	}
 
 	return count;

@@ -560,14 +572,18 @@ static void hidg_disable(struct usb_function *f)
 {
 	struct f_hidg *hidg = func_to_hidg(f);
 	struct f_hidg_req_list *list, *next;
+	unsigned long flags;
 
 	usb_ep_disable(hidg->in_ep);
 	usb_ep_disable(hidg->out_ep);
 
+	spin_lock_irqsave(&hidg->spinlock, flags);
 	list_for_each_entry_safe(list, next, &hidg->completed_out_req, list) {
+		free_ep_req(hidg->out_ep, list->req);
 		list_del(&list->list);
 		kfree(list);
 	}
+	spin_unlock_irqrestore(&hidg->spinlock, flags);
 }
 
 static int hidg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)