[PATCH v3] usb: xhci: Fix memory leak in xhci_disable_slot()

Zilin Guan posted 1 patch 1 month ago
drivers/usb/host/xhci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH v3] usb: xhci: Fix memory leak in xhci_disable_slot()
Posted by Zilin Guan 1 month ago
xhci_alloc_command() allocates a command structure and, when the
second argument is true, also allocates a completion structure.
Currently, the error handling path in xhci_disable_slot() only frees
the command structure using kfree(), causing the completion structure
to leak.

Use xhci_free_command() instead of kfree(). xhci_free_command() correctly
frees both the command structure and the associated completion structure.
Since the command structure is allocated with zero-initialization,
command->in_ctx is NULL and will not be erroneously freed by
xhci_free_command().

This bug was found using an experimental static analysis tool we are
developing. The tool is based on the LLVM framework and is specifically
designed to detect memory management issues. It is currently under
active development and not yet publicly available, but we plan to
open-source it after our research is published.

The bug was originally detected on v6.13-rc1 using our static analysis
tool, and we have verified that the issue persists in the latest mainline
kernel.

We performed build testing on x86_64 with allyesconfig using GCC=11.4.0.
Since triggering these error paths in xhci_disable_slot() requires specific
hardware conditions or abnormal state, we were unable to construct a test
case to reliably trigger these specific error paths at runtime.

Fixes: 7faac1953ed1 ("xhci: avoid race between disable slot command and host runtime suspend")
CC: stable@vger.kernel.org
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
Changes in v3:
- Update the commit message to reflect that the analysis applies to the mainline kernel.

Changes in v2:
- Add detailed information required by researcher guidelines.
- Clarify the safety of using xhci_free_command() in this context.
- Correct the Fixes tag to point to the commit that introduced this issue.

 drivers/usb/host/xhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 02c9bfe21ae2..f0beed054954 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4137,7 +4137,7 @@ int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
 	if (state == 0xffffffff || (xhci->xhc_state & XHCI_STATE_DYING) ||
 			(xhci->xhc_state & XHCI_STATE_HALTED)) {
 		spin_unlock_irqrestore(&xhci->lock, flags);
-		kfree(command);
+		xhci_free_command(xhci, command);
 		return -ENODEV;
 	}
 
@@ -4145,7 +4145,7 @@ int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
 				slot_id);
 	if (ret) {
 		spin_unlock_irqrestore(&xhci->lock, flags);
-		kfree(command);
+		xhci_free_command(xhci, command);
 		return ret;
 	}
 	xhci_ring_cmd_db(xhci);
-- 
2.34.1
Re: [PATCH v3] usb: xhci: Fix memory leak in xhci_disable_slot()
Posted by Michal Pecio 1 week, 3 days ago
On Fri,  9 Jan 2026 04:54:10 +0000, Zilin Guan wrote:
> xhci_alloc_command() allocates a command structure and, when the
> second argument is true, also allocates a completion structure.
> Currently, the error handling path in xhci_disable_slot() only frees
> the command structure using kfree(), causing the completion structure
> to leak.
> 
> Use xhci_free_command() instead of kfree(). xhci_free_command()
> correctly frees both the command structure and the associated
> completion structure. Since the command structure is allocated with
> zero-initialization, command->in_ctx is NULL and will not be
> erroneously freed by xhci_free_command().
> 
> This bug was found using an experimental static analysis tool we are
> developing. The tool is based on the LLVM framework and is
> specifically designed to detect memory management issues. It is
> currently under active development and not yet publicly available,
> but we plan to open-source it after our research is published.
> 
> The bug was originally detected on v6.13-rc1 using our static analysis
> tool, and we have verified that the issue persists in the latest
> mainline kernel.
> 
> We performed build testing on x86_64 with allyesconfig using
> GCC=11.4.0. Since triggering these error paths in xhci_disable_slot()
> requires specific hardware conditions or abnormal state, we were
> unable to construct a test case to reliably trigger these specific
> error paths at runtime.
> 
> Fixes: 7faac1953ed1 ("xhci: avoid race between disable slot command
> and host runtime suspend") CC: stable@vger.kernel.org
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>

This looks like correct fix to an actual bug, but it seems that it has
been missed? I see it neither in usb-next nor usb-linus or mainline.

The leak is still there, even if arguably a small and rare one.

Regards,
Michal
Re: [PATCH v3] usb: xhci: Fix memory leak in xhci_disable_slot()
Posted by Mathias Nyman 1 week, 3 days ago
On 1/29/26 10:56, Michal Pecio wrote:
> On Fri,  9 Jan 2026 04:54:10 +0000, Zilin Guan wrote:
>> xhci_alloc_command() allocates a command structure and, when the
>> second argument is true, also allocates a completion structure.
>> Currently, the error handling path in xhci_disable_slot() only frees
>> the command structure using kfree(), causing the completion structure
>> to leak.
>>
>> Use xhci_free_command() instead of kfree(). xhci_free_command()
>> correctly frees both the command structure and the associated
>> completion structure. Since the command structure is allocated with
>> zero-initialization, command->in_ctx is NULL and will not be
>> erroneously freed by xhci_free_command().
>>
>> This bug was found using an experimental static analysis tool we are
>> developing. The tool is based on the LLVM framework and is
>> specifically designed to detect memory management issues. It is
>> currently under active development and not yet publicly available,
>> but we plan to open-source it after our research is published.
>>
>> The bug was originally detected on v6.13-rc1 using our static analysis
>> tool, and we have verified that the issue persists in the latest
>> mainline kernel.
>>
>> We performed build testing on x86_64 with allyesconfig using
>> GCC=11.4.0. Since triggering these error paths in xhci_disable_slot()
>> requires specific hardware conditions or abnormal state, we were
>> unable to construct a test case to reliably trigger these specific
>> error paths at runtime.
>>
>> Fixes: 7faac1953ed1 ("xhci: avoid race between disable slot command
>> and host runtime suspend") CC: stable@vger.kernel.org
>> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> 
> This looks like correct fix to an actual bug, but it seems that it has
> been missed? I see it neither in usb-next nor usb-linus or mainline.
> 
> The leak is still there, even if arguably a small and rare one.

Thanks for the reminder.
We are so late in the cycle now that I'll send it after rc1

Thanks
Mathias