There is a possible null-pointer dereference related to the wait-completion
synchronization mechanism in the function xhci_setup_device().
Consider the following execution scenario:
in drivers/usb/host/xhci-mem.c:
xhci_mem_init() // 2381
if (!xhci->dcbaa) // 2431 xhci->dcbaa can be NULL
xhci_mem_cleanup() // 2548
xhci_cleanup_command_queue() // 1888
in drivers/usb/host/xhci-ring.c
xhci_complete_del_and_free_cmd() // 1619
complete(cmd->completion); // 1608
The variable xhci->dcbaa is checked by an if statement at Line 2431. If
xhci->dcbaa is NULL, xhci_mem_cleanup() will be called at Line 2548, which
eventually leads to complete() at Line 1608 that wakes up the
wait_for_completion().
Consider the wait_for_completion() in drivers/usb/host/xhci.c
xhci_setup_device()
wait_for_completion(command->completion); // 4179
le64_to_cpu(xhci->dcbaa->dev_context_ptrs...)); // 4237
The variable xhci->dcbaa is dereferenced (without being rechecked) after
the wait_for_completion(), which can introduce a possible null-pointer
dereference.
To address this issue, a NULL check is added to ensure the variable
xhci->dcbaa is not NULL when xhci_dbg_trace() is called.
This potential bug was discovered using an experimental static analysis
tool developed by our team. The tool deduces complete() and
wait_for_completion() pairs using alias analysis. It then applies data
flow analysis to detect null-pointer dereferences across synchronization
points.
Fixes: 84a99f6fc5d4 ("xhci: add traces for debug messages in xhci_address_device()")
Signed-off-by: Tuo Li <islituo@xxxxxxxxx>
---
drivers/usb/host/xhci.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 899c0effb5d3..1c7322a4e301 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4229,12 +4229,14 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
temp_64 = xhci_read_64(xhci, &xhci->op_regs->dcbaa_ptr);
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
"Op regs DCBAA ptr = %#016llx", temp_64);
- xhci_dbg_trace(xhci, trace_xhci_dbg_address,
- "Slot ID %d dcbaa entry @%p = %#016llx",
- udev->slot_id,
- &xhci->dcbaa->dev_context_ptrs[udev->slot_id],
- (unsigned long long)
- le64_to_cpu(xhci->dcbaa->dev_context_ptrs[udev->slot_id]));
+ if (xhci->dcbaa) {
+ xhci_dbg_trace(xhci, trace_xhci_dbg_address,
+ "Slot ID %d dcbaa entry @%p = %#016llx",
+ udev->slot_id,
+ &xhci->dcbaa->dev_context_ptrs[udev->slot_id],
+ (unsigned long long)
+ le64_to_cpu(xhci->dcbaa->dev_context_ptrs[udev->slot_id]));
+ }
xhci_dbg_trace(xhci, trace_xhci_dbg_address,
"Output Context DMA address = %#08llx",
(unsigned long long)virt_dev->out_ctx->dma);
--
2.43.0
