Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] xhci: fix memleak on setup address fails.
@ 2019-08-11  8:22 Ikjoon Jang
  2019-08-14 13:59 ` Mathias Nyman
  0 siblings, 1 reply; 2+ messages in thread
From: Ikjoon Jang @ 2019-08-11  8:22 UTC (permalink / raw)
  To: Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel, Ikjoon Jang

Xhci re-enables a slot on transaction error in set_address using
xhci_disable_slot() + xhci_alloc_dev().

But in this case, xhci_alloc_dev() creates debugfs entries upon an
existing device without cleaning up old entries, thus memory leaks.

So this patch simply moves calling xhci_debugfs_free_dev() from
xhci_free_dev() to xhci_disable_slot().

Signed-off-by: Ikjoon Jang <ikjn@chromium.org>
---
 drivers/usb/host/xhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 03d1e552769b..c24c5bf9ef9c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3814,7 +3814,6 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
 		virt_dev->eps[i].ep_state &= ~EP_STOP_CMD_PENDING;
 		del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
 	}
-	xhci_debugfs_remove_slot(xhci, udev->slot_id);
 	virt_dev->udev = NULL;
 	ret = xhci_disable_slot(xhci, udev->slot_id);
 	if (ret)
@@ -3832,6 +3831,8 @@ int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
 	if (!command)
 		return -ENOMEM;
 
+	xhci_debugfs_remove_slot(xhci, slot_id);
+
 	spin_lock_irqsave(&xhci->lock, flags);
 	/* Don't disable the slot if the host controller is dead. */
 	state = readl(&xhci->op_regs->status);
-- 
2.23.0.rc1.153.gdeed80330f-goog


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] xhci: fix memleak on setup address fails.
  2019-08-11  8:22 [PATCH] xhci: fix memleak on setup address fails Ikjoon Jang
@ 2019-08-14 13:59 ` Mathias Nyman
  0 siblings, 0 replies; 2+ messages in thread
From: Mathias Nyman @ 2019-08-14 13:59 UTC (permalink / raw)
  To: Ikjoon Jang, Mathias Nyman, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel

On 11.8.2019 11.22, Ikjoon Jang wrote:
> Xhci re-enables a slot on transaction error in set_address using
> xhci_disable_slot() + xhci_alloc_dev().
> 
> But in this case, xhci_alloc_dev() creates debugfs entries upon an
> existing device without cleaning up old entries, thus memory leaks.
> 
> So this patch simply moves calling xhci_debugfs_free_dev() from
> xhci_free_dev() to xhci_disable_slot().
> 

Othwerwise this looks good, but xhci_alloc_dev() will call xhci_disable_slot()
in some failure cases before the slot debugfs entry is created.

In these cases xhci_debugfs_remove_slot() will be called without
xhci_debugfs_create_slot() ever being called.

This might not be an issue as xhci_debugfs_remove_slot() checks
if (!dev || !dev->debugfs_private) before doing anything, but should
be checked out.

-Mathias

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-11  8:22 [PATCH] xhci: fix memleak on setup address fails Ikjoon Jang
2019-08-14 13:59 ` Mathias Nyman

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org linux-usb@archiver.kernel.org
	public-inbox-index linux-usb


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/ public-inbox