Commit 50985e6d authored by Bryan O'Donoghue's avatar Bryan O'Donoghue Committed by Khalid Elmously

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

BugLink: https://bugs.launchpad.net/bugs/1864775

commit 5b24c28c upstream.

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: 40d133d7 ("usb: gadget: f_ncm: convert to new function interface with backward compatibility")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: default avatarBryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: default avatarFelipe Balbi <balbi@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
Signed-off-by: default avatarKleber Sacilotto de Souza <kleber.souza@canonical.com>
parent b52e6db1
...@@ -57,6 +57,7 @@ struct f_ncm { ...@@ -57,6 +57,7 @@ struct f_ncm {
struct usb_ep *notify; struct usb_ep *notify;
struct usb_request *notify_req; struct usb_request *notify_req;
u8 notify_state; u8 notify_state;
atomic_t notify_count;
bool is_open; bool is_open;
const struct ndp_parser_opts *parser_opts; const struct ndp_parser_opts *parser_opts;
...@@ -480,7 +481,7 @@ static void ncm_do_notify(struct f_ncm *ncm) ...@@ -480,7 +481,7 @@ static void ncm_do_notify(struct f_ncm *ncm)
int status; int status;
/* notification already in flight? */ /* notification already in flight? */
if (!req) if (atomic_read(&ncm->notify_count))
return; return;
event = req->buf; event = req->buf;
...@@ -520,7 +521,8 @@ static void ncm_do_notify(struct f_ncm *ncm) ...@@ -520,7 +521,8 @@ static void ncm_do_notify(struct f_ncm *ncm)
event->bmRequestType = 0xA1; event->bmRequestType = 0xA1;
event->wIndex = cpu_to_le16(ncm->ctrl_id); 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, * In double buffering if there is a space in FIFO,
* completion callback can be called right after the call, * completion callback can be called right after the call,
...@@ -530,7 +532,7 @@ static void ncm_do_notify(struct f_ncm *ncm) ...@@ -530,7 +532,7 @@ static void ncm_do_notify(struct f_ncm *ncm)
status = usb_ep_queue(ncm->notify, req, GFP_ATOMIC); status = usb_ep_queue(ncm->notify, req, GFP_ATOMIC);
spin_lock(&ncm->lock); spin_lock(&ncm->lock);
if (status < 0) { if (status < 0) {
ncm->notify_req = req; atomic_dec(&ncm->notify_count);
DBG(cdev, "notify --> %d\n", status); DBG(cdev, "notify --> %d\n", status);
} }
} }
...@@ -565,17 +567,19 @@ static void ncm_notify_complete(struct usb_ep *ep, struct usb_request *req) ...@@ -565,17 +567,19 @@ static void ncm_notify_complete(struct usb_ep *ep, struct usb_request *req)
case 0: case 0:
VDBG(cdev, "Notification %02x sent\n", VDBG(cdev, "Notification %02x sent\n",
event->bNotificationType); event->bNotificationType);
atomic_dec(&ncm->notify_count);
break; break;
case -ECONNRESET: case -ECONNRESET:
case -ESHUTDOWN: case -ESHUTDOWN:
atomic_set(&ncm->notify_count, 0);
ncm->notify_state = NCM_NOTIFY_NONE; ncm->notify_state = NCM_NOTIFY_NONE;
break; break;
default: default:
DBG(cdev, "event %02x --> %d\n", DBG(cdev, "event %02x --> %d\n",
event->bNotificationType, req->status); event->bNotificationType, req->status);
atomic_dec(&ncm->notify_count);
break; break;
} }
ncm->notify_req = req;
ncm_do_notify(ncm); ncm_do_notify(ncm);
spin_unlock(&ncm->lock); spin_unlock(&ncm->lock);
} }
...@@ -1559,6 +1563,11 @@ static void ncm_unbind(struct usb_configuration *c, struct usb_function *f) ...@@ -1559,6 +1563,11 @@ static void ncm_unbind(struct usb_configuration *c, struct usb_function *f)
ncm_string_defs[0].id = 0; ncm_string_defs[0].id = 0;
usb_free_all_descriptors(f); 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); kfree(ncm->notify_req->buf);
usb_ep_free_request(ncm->notify, ncm->notify_req); usb_ep_free_request(ncm->notify, ncm->notify_req);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment