From 7d602f30149a117eea260208b1661bc404c21dfd Mon Sep 17 00:00:00 2001 From: Frank Li Date: Wed, 8 Jun 2022 14:04:30 -0500 Subject: usb: cdns3 fix use-after-free at workaround 2 BUG: KFENCE: use-after-free read in __list_del_entry_valid+0x10/0xac cdns3_wa2_remove_old_request() { ... kfree(priv_req->request.buf); cdns3_gadget_ep_free_request(&priv_ep->endpoint, &priv_req->request); list_del_init(&priv_req->list); ^^^ use after free ... } cdns3_gadget_ep_free_request() free the space pointed by priv_req, but priv_req is used in the following list_del_init(). This patch move list_del_init() before cdns3_gadget_ep_free_request(). Signed-off-by: Frank Li Signed-off-by: Faqiang Zhu Link: https://lore.kernel.org/r/20220608190430.2814358-1-Frank.Li@nxp.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/cdns3/cdns3-gadget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/cdns3/cdns3-gadget.c') diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c index 5c15c48952a6..29662c8ac024 100644 --- a/drivers/usb/cdns3/cdns3-gadget.c +++ b/drivers/usb/cdns3/cdns3-gadget.c @@ -625,9 +625,9 @@ static void cdns3_wa2_remove_old_request(struct cdns3_endpoint *priv_ep) trace_cdns3_wa2(priv_ep, "removes eldest request"); kfree(priv_req->request.buf); + list_del_init(&priv_req->list); cdns3_gadget_ep_free_request(&priv_ep->endpoint, &priv_req->request); - list_del_init(&priv_req->list); --priv_ep->wa2_counter; if (!chain) -- cgit v1.2.3 From 8659ab3d936fcf0084676f98b75b317017aa8f82 Mon Sep 17 00:00:00 2001 From: Frank Li Date: Thu, 9 Jun 2022 10:44:56 -0500 Subject: usb: cdns3: fix random warning message when driver load Warning log: [ 4.141392] Unexpected gfp: 0x4 (GFP_DMA32). Fixing up to gfp: 0xa20 (GFP_ATOMIC). Fix your code! [ 4.150340] CPU: 1 PID: 175 Comm: 1-0050 Not tainted 5.15.5-00039-g2fd9ae1b568c #20 [ 4.158010] Hardware name: Freescale i.MX8QXP MEK (DT) [ 4.163155] Call trace: [ 4.165600] dump_backtrace+0x0/0x1b0 [ 4.169286] show_stack+0x18/0x68 [ 4.172611] dump_stack_lvl+0x68/0x84 [ 4.176286] dump_stack+0x18/0x34 [ 4.179613] kmalloc_fix_flags+0x60/0x88 [ 4.183550] new_slab+0x334/0x370 [ 4.186878] ___slab_alloc.part.108+0x4d4/0x748 [ 4.191419] __slab_alloc.isra.109+0x30/0x78 [ 4.195702] kmem_cache_alloc+0x40c/0x420 [ 4.199725] dma_pool_alloc+0xac/0x1f8 [ 4.203486] cdns3_allocate_trb_pool+0xb4/0xd0 pool_alloc_page(struct dma_pool *pool, gfp_t mem_flags) { ... page = kmalloc(sizeof(*page), mem_flags); page->vaddr = dma_alloc_coherent(pool->dev, pool->allocation, &page->dma, mem_flags); ... } kmalloc was called with mem_flags, which is passed down in cdns3_allocate_trb_pool() and have GFP_DMA32 flags. kmall_fix_flags() report warning. GFP_DMA32 is not useful at all. dma_alloc_coherent() will handle DMA memory region correctly by pool->dev. GFP_DMA32 can be removed safely. Signed-off-by: Frank Li Link: https://lore.kernel.org/r/20220609154456.2871672-1-Frank.Li@nxp.com Signed-off-by: Greg Kroah-Hartman --- drivers/usb/cdns3/cdns3-gadget.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/cdns3/cdns3-gadget.c') diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c index 29662c8ac024..555caafe4f04 100644 --- a/drivers/usb/cdns3/cdns3-gadget.c +++ b/drivers/usb/cdns3/cdns3-gadget.c @@ -220,7 +220,7 @@ int cdns3_allocate_trb_pool(struct cdns3_endpoint *priv_ep) if (!priv_ep->trb_pool) { priv_ep->trb_pool = dma_pool_alloc(priv_dev->eps_dma_pool, - GFP_DMA32 | GFP_ATOMIC, + GFP_ATOMIC, &priv_ep->trb_pool_dma); if (!priv_ep->trb_pool) -- cgit v1.2.3 From c3ffc9c4ca44bfe9562166793d133e1fb0630ea6 Mon Sep 17 00:00:00 2001 From: Andrey Strachuk Date: Mon, 18 Jul 2022 19:00:52 +0300 Subject: usb: cdns3: change place of 'priv_ep' assignment in cdns3_gadget_ep_dequeue(), cdns3_gadget_ep_enable() If 'ep' is NULL, result of ep_to_cdns3_ep(ep) is invalid pointer and its dereference with priv_ep->cdns3_dev may cause panic. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver") Acked-by: Peter Chen Signed-off-by: Andrey Strachuk Link: https://lore.kernel.org/r/20220718160052.4188-1-strochuk@ispras.ru Signed-off-by: Greg Kroah-Hartman --- drivers/usb/cdns3/cdns3-gadget.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'drivers/usb/cdns3/cdns3-gadget.c') diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c index 555caafe4f04..9ac7d0a8c5da 100644 --- a/drivers/usb/cdns3/cdns3-gadget.c +++ b/drivers/usb/cdns3/cdns3-gadget.c @@ -2285,14 +2285,15 @@ static int cdns3_gadget_ep_enable(struct usb_ep *ep, int val; priv_ep = ep_to_cdns3_ep(ep); - priv_dev = priv_ep->cdns3_dev; - comp_desc = priv_ep->endpoint.comp_desc; if (!ep || !desc || desc->bDescriptorType != USB_DT_ENDPOINT) { dev_dbg(priv_dev->dev, "usbss: invalid parameters\n"); return -EINVAL; } + comp_desc = priv_ep->endpoint.comp_desc; + priv_dev = priv_ep->cdns3_dev; + if (!desc->wMaxPacketSize) { dev_err(priv_dev->dev, "usbss: missing wMaxPacketSize\n"); return -EINVAL; @@ -2600,7 +2601,7 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep, struct usb_request *request) { struct cdns3_endpoint *priv_ep = ep_to_cdns3_ep(ep); - struct cdns3_device *priv_dev = priv_ep->cdns3_dev; + struct cdns3_device *priv_dev; struct usb_request *req, *req_temp; struct cdns3_request *priv_req; struct cdns3_trb *link_trb; @@ -2611,6 +2612,8 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep, if (!ep || !request || !ep->desc) return -EINVAL; + priv_dev = priv_ep->cdns3_dev; + spin_lock_irqsave(&priv_dev->lock, flags); priv_req = to_cdns3_request(request); -- cgit v1.2.3 From 78acd4ca433425e6dd4032cfc2156c60e34931f2 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Wed, 3 Aug 2022 09:24:22 -0700 Subject: usb: cdns3: Don't use priv_dev uninitialized in cdns3_gadget_ep_enable() Clang warns: drivers/usb/cdns3/cdns3-gadget.c:2290:11: error: variable 'priv_dev' is uninitialized when used here [-Werror,-Wuninitialized] dev_dbg(priv_dev->dev, "usbss: invalid parameters\n"); ^~~~~~~~ include/linux/dev_printk.h:155:18: note: expanded from macro 'dev_dbg' dynamic_dev_dbg(dev, dev_fmt(fmt), ##__VA_ARGS__) ^~~ include/linux/dynamic_debug.h:167:7: note: expanded from macro 'dynamic_dev_dbg' dev, fmt, ##__VA_ARGS__) ^~~ include/linux/dynamic_debug.h:152:56: note: expanded from macro '_dynamic_func_call' __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) ^~~~~~~~~~~ include/linux/dynamic_debug.h:134:15: note: expanded from macro '__dynamic_func_call' func(&id, ##__VA_ARGS__); \ ^~~~~~~~~~~ drivers/usb/cdns3/cdns3-gadget.c:2278:31: note: initialize the variable 'priv_dev' to silence this warning struct cdns3_device *priv_dev; ^ = NULL 1 error generated. The priv_dev assignment was moved below the if statement to avoid potentially dereferencing ep before it was checked but priv_dev is used in the dev_dbg() call. To fix this, move the priv_dev and comp_desc assignments back to their original spot and hoist the ep check above those assignments with a call to pr_debug() instead of dev_dbg(). Fixes: c3ffc9c4ca44 ("usb: cdns3: change place of 'priv_ep' assignment in cdns3_gadget_ep_dequeue(), cdns3_gadget_ep_enable()") Link: https://github.com/ClangBuiltLinux/linux/issues/1680 Signed-off-by: Nathan Chancellor Signed-off-by: Linus Torvalds --- drivers/usb/cdns3/cdns3-gadget.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/usb/cdns3/cdns3-gadget.c') diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c index 9ac7d0a8c5da..d21b69997e75 100644 --- a/drivers/usb/cdns3/cdns3-gadget.c +++ b/drivers/usb/cdns3/cdns3-gadget.c @@ -2284,16 +2284,20 @@ static int cdns3_gadget_ep_enable(struct usb_ep *ep, int ret = 0; int val; + if (!ep) { + pr_debug("usbss: ep not configured?\n"); + return -EINVAL; + } + priv_ep = ep_to_cdns3_ep(ep); + priv_dev = priv_ep->cdns3_dev; + comp_desc = priv_ep->endpoint.comp_desc; - if (!ep || !desc || desc->bDescriptorType != USB_DT_ENDPOINT) { + if (!desc || desc->bDescriptorType != USB_DT_ENDPOINT) { dev_dbg(priv_dev->dev, "usbss: invalid parameters\n"); return -EINVAL; } - comp_desc = priv_ep->endpoint.comp_desc; - priv_dev = priv_ep->cdns3_dev; - if (!desc->wMaxPacketSize) { dev_err(priv_dev->dev, "usbss: missing wMaxPacketSize\n"); return -EINVAL; -- cgit v1.2.3