Skip to contentSkip to conversation

patch "usb: gadget: f_ncm: Use atomic_t to track in-flight request" added to usb-next

PATCH [email protected] 1 participant 1 message Jan 15, 2020

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

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

usb: gadget: f_ncm: Use atomic_t to track in-flight request

to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-next branch.

The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)

The patch will also be merged in the next major kernel release during the merge window.

If you have any questions about this process, please let me know.

From 5b24c28cfe136597dc3913e1c00b119307a20c7e Mon Sep 17 00:00:00 2001
From: Bryan O'Donoghue <[email protected]>
Date: Thu, 9 Jan 2020 13:17:21 +0000
Subject: usb: gadget: f_ncm: Use atomic_t to track in-flight request

Currently ncm->notify_req is used to flag when a request is in-flight. ncm->notify_req is set to NULL and when a request completes it is subsequently reset.

This is fundamentally buggy in that the unbind logic of the NCM driver will unconditionally free ncm->notify_req leading to a NULL pointer dereference.

Fixes 40d133d7f542 ("usb: gadget: f_ncm: convert to new function interface with backward compatibility")
Cc stable <[email protected]>
Signed-off-by Bryan O'Donoghue <[email protected]>
Signed-off-by Felipe Balbi <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/usb/gadget/function/f_ncm.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
drivers/usb/gadget/function/f_ncm.c +13 -4
index 2d6e76e4cffa..1d900081b1f0 100644
@@ -53,6 +53,7 @@ struct f_ncm {
 	struct usb_ep			*notify;
 	struct usb_request		*notify_req;
 	u8				notify_state;
+	atomic_t			notify_count;
 	bool				is_open;
 
 	const struct ndp_parser_opts	*parser_opts;

@@ -547,7 +548,7 @@ static void ncm_do_notify(struct f_ncm *ncm)
 	int				status;
 
 	/* notification already in flight? */
-	if (!req)
+	if (atomic_read(&ncm->notify_count))
 		return;
 
 	event = req->buf;

@@ -587,7 +588,8 @@ static void ncm_do_notify(struct f_ncm *ncm)
 	event->bmRequestType = 0xA1;
 	event->wIndex = cpu_to_le16(ncm->ctrl_id);
 
-	ncm->notify_req = NULL;
+	atomic_inc(&ncm->notify_count);
+
 	/*
 	 * In double buffering if there is a space in FIFO,
 	 * completion callback can be called right after the call,

@@ -597,7 +599,7 @@ static void ncm_do_notify(struct f_ncm *ncm)
 	status = usb_ep_queue(ncm->notify, req, GFP_ATOMIC);
 	spin_lock(&ncm->lock);
 	if (status < 0) {
-		ncm->notify_req = req;
+		atomic_dec(&ncm->notify_count);
 		DBG(cdev, "notify --> %d\n", status);
 	}
 }

@@ -632,17 +634,19 @@ static void ncm_notify_complete(struct usb_ep *ep, struct usb_request *req)
 	case 0:
 		VDBG(cdev, "Notification %02x sent\n",
 		     event->bNotificationType);
+		atomic_dec(&ncm->notify_count);
 		break;
 	case -ECONNRESET:
 	case -ESHUTDOWN:
+		atomic_set(&ncm->notify_count, 0);
 		ncm->notify_state = NCM_NOTIFY_NONE;
 		break;
 	default:
 		DBG(cdev, "event %02x --> %d\n",
 			event->bNotificationType, req->status);
+		atomic_dec(&ncm->notify_count);
 		break;
 	}
-	ncm->notify_req = req;
 	ncm_do_notify(ncm);
 	spin_unlock(&ncm->lock);
 }

@@ -1649,6 +1653,11 @@ static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)
 	ncm_string_defs[0].id = 0;
 	usb_free_all_descriptors(f);
 
+	if (atomic_read(&ncm->notify_count)) {
+		usb_ep_dequeue(ncm->notify, ncm->notify_req);
+		atomic_set(&ncm->notify_count, 0);
+	}
+
 	kfree(ncm->notify_req->buf);
 	usb_ep_free_request(ncm->notify, ncm->notify_req);
 }
Signature · 1 line
2.24.1