From b03daf57bc0cf33579158358e7a25636d02e1178 Mon Sep 17 00:00:00 2001 From: Udipto Goswami Date: Thu, 13 Feb 2020 12:26:51 +0530 Subject: [PATCH] usb: gadget: Prevent use after free in qdss connect & close Dwc3 gadget controller driver can still access usb_request after dequeue() returns due to async cancellation. Hence usb_request shouldn't be freed after calling usb_ep_dequeue() which can otherwise cause use after free. Fix this by allocating & de-allocating from bind and unbind instead of data_connection & close. Change-Id: I8155ecbebcf762fc87c17841c72c53c6dca2e4a9 Signed-off-by: Udipto Goswami Signed-off-by: Mayank Rana --- drivers/usb/gadget/function/f_qdss.c | 47 +++++++++++++++++----------- drivers/usb/gadget/function/f_qdss.h | 3 +- drivers/usb/gadget/function/u_qdss.c | 5 ++- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/drivers/usb/gadget/function/f_qdss.c b/drivers/usb/gadget/function/f_qdss.c index 7b244c374bbf..ae0180b6e6f8 100644 --- a/drivers/usb/gadget/function/f_qdss.c +++ b/drivers/usb/gadget/function/f_qdss.c @@ -441,7 +441,7 @@ static int qdss_bind(struct usb_configuration *c, struct usb_function *f) &qdss_data_ep_comp_desc); if (!ep) { pr_err("%s: ep_autoconfig error\n", __func__); - goto fail; + goto clear_ep; } qdss->port.data = ep; ep->driver_data = qdss; @@ -454,8 +454,9 @@ static int qdss_bind(struct usb_configuration *c, struct usb_function *f) &qdss_ctrl_in_ep_comp_desc); if (!ep) { pr_err("%s: ep_autoconfig error\n", __func__); - goto fail; + goto clear_ep; } + qdss->port.ctrl_in = ep; ep->driver_data = qdss; @@ -463,12 +464,21 @@ static int qdss_bind(struct usb_configuration *c, struct usb_function *f) &qdss_ctrl_out_ep_comp_desc); if (!ep) { pr_err("%s: ep_autoconfig error\n", __func__); - goto fail; + goto clear_ep; } qdss->port.ctrl_out = ep; ep->driver_data = qdss; } + if (!strcmp(qdss->ch.name, USB_QDSS_CH_MSM)) { + ret = alloc_sps_req(qdss->port.data); + if (ret) { + pr_err("%s: alloc_sps_req error (%d)\n", + __func__, ret); + goto clear_ep; + } + } + /*update fs descriptors*/ qdss_fs_data_desc.bEndpointAddress = qdss_ss_data_desc.bEndpointAddress; @@ -501,9 +511,18 @@ static int qdss_bind(struct usb_configuration *c, struct usb_function *f) goto fail; return 0; + fail: + /* check if usb_request allocated */ + if (qdss->endless_req) { + usb_ep_free_request(qdss->port.data, + qdss->endless_req); + qdss->endless_req = NULL; + } + +clear_ep: clear_eps(f); - clear_desc(gadget, f); + return -ENOTSUPP; } @@ -517,6 +536,12 @@ static void qdss_unbind(struct usb_configuration *c, struct usb_function *f) flush_workqueue(qdss->wq); + if (qdss->endless_req) { + usb_ep_free_request(qdss->port.data, + qdss->endless_req); + qdss->endless_req = NULL; + } + /* Reset string ids */ qdss_string_defs[QDSS_DATA_IDX].id = 0; qdss_string_defs[QDSS_CTRL_IDX].id = 0; @@ -553,7 +578,6 @@ static void usb_qdss_disconnect_work(struct work_struct *work) { struct f_qdss *qdss; int status; - unsigned long flags; qdss = container_of(work, struct f_qdss, disconnect_w); pr_debug("%s\n", __func__); @@ -575,14 +599,6 @@ static void usb_qdss_disconnect_work(struct work_struct *work) status = set_qdss_data_connection(qdss, 0); if (status) pr_err("qdss_disconnect error\n"); - - spin_lock_irqsave(&qdss->lock, flags); - if (qdss->endless_req) { - usb_ep_free_request(qdss->port.data, - qdss->endless_req); - qdss->endless_req = NULL; - } - spin_unlock_irqrestore(&qdss->lock, flags); } /* @@ -1005,11 +1021,6 @@ void usb_qdss_close(struct usb_qdss_ch *ch) spin_unlock_irqrestore(&qdss_lock, flags); usb_ep_dequeue(qdss->port.data, qdss->endless_req); spin_lock_irqsave(&qdss_lock, flags); - if (qdss->endless_req) { - usb_ep_free_request(qdss->port.data, - qdss->endless_req); - qdss->endless_req = NULL; - } } gadget = qdss->gadget; ch->app_conn = 0; diff --git a/drivers/usb/gadget/function/f_qdss.h b/drivers/usb/gadget/function/f_qdss.h index 1b5c91a6c717..13186d6e8988 100644 --- a/drivers/usb/gadget/function/f_qdss.h +++ b/drivers/usb/gadget/function/f_qdss.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ /* - * Copyright (c) 2012-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved. */ #ifndef _F_QDSS_H @@ -72,4 +72,5 @@ struct usb_qdss_opts { int uninit_data(struct usb_ep *ep); int set_qdss_data_connection(struct f_qdss *qdss, int enable); +int alloc_sps_req(struct usb_ep *data_ep); #endif diff --git a/drivers/usb/gadget/function/u_qdss.c b/drivers/usb/gadget/function/u_qdss.c index 41862845f417..5556ed2d992c 100644 --- a/drivers/usb/gadget/function/u_qdss.c +++ b/drivers/usb/gadget/function/u_qdss.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2012-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2012-2020, The Linux Foundation. All rights reserved. */ #include @@ -10,7 +10,7 @@ #include #include "f_qdss.h" -static int alloc_sps_req(struct usb_ep *data_ep) +int alloc_sps_req(struct usb_ep *data_ep) { struct usb_request *req = NULL; struct f_qdss *qdss = data_ep->driver_data; @@ -95,7 +95,6 @@ int set_qdss_data_connection(struct f_qdss *qdss, int enable) &bam_info.usb_bam_pipe_idx, NULL, bam_info.data_fifo, NULL); - alloc_sps_req(qdss->port.data); msm_data_fifo_config(qdss->port.data, bam_info.data_fifo->iova, bam_info.data_fifo->size,