Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752176AbdFAQRq (ORCPT ); Thu, 1 Jun 2017 12:17:46 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:33273 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752075AbdFAPo7 (ORCPT ); Thu, 1 Jun 2017 11:44:59 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "David Lechner" , "Krzysztof Opasiak" , "Krzysztof Opasiak" , "Felipe Balbi" Date: Thu, 01 Jun 2017 16:43:16 +0100 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.16 068/212] usb: gadget: f_hid: fix: Prevent accessing released memory In-Reply-To: X-SA-Exim-Connect-IP: 82.70.136.246 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2925 Lines: 90 3.16.44-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Krzysztof Opasiak commit aa65d11aa008f4de58a9cee7e121666d9d68505e upstream. 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") Tested-by: David Lechner Signed-off-by: Krzysztof Opasiak Signed-off-by: Felipe Balbi [bwh: Backported to 3.16: adjust filename, context] Signed-off-by: Ben Hutchings --- drivers/usb/gadget/f_hid.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) --- a/drivers/usb/gadget/f_hid.c +++ b/drivers/usb/gadget/f_hid.c @@ -197,6 +197,13 @@ static ssize_t f_hidg_read(struct file * /* 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); @@ -212,15 +219,20 @@ static ssize_t f_hidg_read(struct file * * 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; @@ -471,6 +483,7 @@ static void hidg_disable(struct usb_func { struct f_hidg *hidg = func_to_hidg(f); struct f_hidg_req_list *list, *next; + unsigned long flags; usb_ep_disable(hidg->in_ep); hidg->in_ep->driver_data = NULL; @@ -478,10 +491,13 @@ static void hidg_disable(struct usb_func usb_ep_disable(hidg->out_ep); hidg->out_ep->driver_data = NULL; + 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)