From: Bryan O'Donoghue bryan.odonoghue@linaro.org
commit 5b24c28cfe136597dc3913e1c00b119307a20c7e 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: 40d133d7f542 ("usb: gadget: f_ncm: convert to new function interface with backward compatibility") Cc: stable stable@vger.kernel.org Signed-off-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Signed-off-by: Felipe Balbi balbi@kernel.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/usb/gadget/function/f_ncm.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
--- a/drivers/usb/gadget/function/f_ncm.c +++ b/drivers/usb/gadget/function/f_ncm.c @@ -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 * 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 * 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 * 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 u 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_config 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); }