linux-kernel - [PATCH] usb: xhci: fix a possible null-pointer dereference in xhci_setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241001194526.25757-1-islituo@gmail.com>
Date: Wed,  2 Oct 2024 05:45:26 +1000
From: Tuo Li <islituo@...il.com>
To: mathias.nyman@...el.com,
	gregkh@...uxfoundation.org
Cc: linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	baijiaju1990@...il.com,
	Tuo Li <islituo@...il.com>
Subject: [PATCH] usb: xhci: fix a possible null-pointer dereference in xhci_setup_device()

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@...il.com>
---
 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