[PATCH] usb: xhci: Check kcalloc_node() when allocating interrupter array in xhci_mem_init()
kcalloc_node() may fail. When the interrupter array allocation returns NULL, subsequent code uses xhci->interrupters (e.g. in xhci_add_interrupter() and in cleanup paths), leading to a potential NULL pointer dereference.
Check the allocation and bail out to the existing fail path to avoid the NULL dereference.
Fixes: c99b38c412343 ("xhci: add support to allocate several interrupters") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li lgs201920130244@gmail.com --- drivers/usb/host/xhci-mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index d698095fc88d..da257856e864 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2505,7 +2505,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) "Allocating primary event ring"); xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters), flags, dev_to_node(dev)); - + if (!xhci->interrupters) + goto fail; ir = xhci_alloc_interrupter(xhci, 0, flags); if (!ir) goto fail;
On Thu, 18 Sep 2025 21:08:38 +0800, Guangshuo Li wrote:
kcalloc_node() may fail. When the interrupter array allocation returns NULL, subsequent code uses xhci->interrupters (e.g. in xhci_add_interrupter() and in cleanup paths), leading to a potential NULL pointer dereference.
Check the allocation and bail out to the existing fail path to avoid the NULL dereference.
Fixes: c99b38c412343 ("xhci: add support to allocate several interrupters") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li lgs201920130244@gmail.com
drivers/usb/host/xhci-mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index d698095fc88d..da257856e864 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2505,7 +2505,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) "Allocating primary event ring"); xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters), flags, dev_to_node(dev));
- if (!xhci->interrupters)
goto fail;
This is a patch against some old kernel, this exact check has been added in v6.16 by 83d98dea48eb6.
But it's missing Fixes and Cc:stable, so 6.6 and 6.12 were left broken.
ir = xhci_alloc_interrupter(xhci, 0, flags); if (!ir) goto fail; -- 2.43.0
On Thu, 18 Sep 2025 21:08:38 +0800, Guangshuo Li wrote:
kcalloc_node() may fail. When the interrupter array allocation returns NULL, subsequent code uses xhci->interrupters (e.g. in xhci_add_interrupter() and in cleanup paths), leading to a potential NULL pointer dereference.
Check the allocation and bail out to the existing fail path to avoid the NULL dereference.
Fixes: c99b38c412343 ("xhci: add support to allocate several interrupters") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li lgs201920130244@gmail.com
drivers/usb/host/xhci-mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index d698095fc88d..da257856e864 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2505,7 +2505,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) "Allocating primary event ring"); xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters), flags, dev_to_node(dev));
- if (!xhci->interrupters)
goto fail;-- 2.43.0
Hi Greg and Mathias,
I noticed that this bug still exists in current 6.6 and 6.12 releases, what would be the sensible course of action to fix it?
The patch from Guangshuo Li is a specific fix and it applies cleanly on those branches. By simulating allocation failure, I confirmed the bug and the fix on 6.6.113, which is identical to the current 6.6.116.
Mainline added an identical check in 83d98dea48eb ("usb: xhci: add individual allocation checks in xhci_mem_init()") which is a cleanup that has a merge conflict at least with 6.6.
The conflict seems superficial and probably the cleanup would have no side effects on 6.6/6.12, but I haven't really reviewed this and maybe it would be simpler to just take the targeted fix?
Regards, Michal
On 11/3/25 10:40, Michal Pecio wrote:
On Thu, 18 Sep 2025 21:08:38 +0800, Guangshuo Li wrote:
kcalloc_node() may fail. When the interrupter array allocation returns NULL, subsequent code uses xhci->interrupters (e.g. in xhci_add_interrupter() and in cleanup paths), leading to a potential NULL pointer dereference.
Check the allocation and bail out to the existing fail path to avoid the NULL dereference.
Fixes: c99b38c412343 ("xhci: add support to allocate several interrupters") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li lgs201920130244@gmail.com
drivers/usb/host/xhci-mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index d698095fc88d..da257856e864 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2505,7 +2505,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) "Allocating primary event ring"); xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters), flags, dev_to_node(dev));
- if (!xhci->interrupters)
goto fail;-- 2.43.0
Hi Greg and Mathias,
I noticed that this bug still exists in current 6.6 and 6.12 releases, what would be the sensible course of action to fix it?
Not sure this qualifies for stable. Is this something that has really happened in real life?
The stable-kernel-rules.rst states it should "fix a real bug that bothers people"
If kcalloc_node() fails to allocate that array of pointers then something else is already badly messed up.
That being said, I don't object this being added to stable either
Thanks Mathias
On Mon, 3 Nov 2025 13:02:06 +0200, Mathias Nyman wrote:
Hi Greg and Mathias,
I noticed that this bug still exists in current 6.6 and 6.12 releases, what would be the sensible course of action to fix it?
Not sure this qualifies for stable. Is this something that has really happened in real life?
The stable-kernel-rules.rst states it should "fix a real bug that bothers people"
If kcalloc_node() fails to allocate that array of pointers then something else is already badly messed up.
I don't know how the reported found it, but it can obviously happen when the driver is bound to a new xHCI controller under OOM conditions.
So maybe not very often, but xHCI hotplug is a thing in Thunderbolt and OOM happens sometimes too, so it's not exactly impossible either.
I thought it's usual to fix such bugs when they are known. Simulated allocation failure before/after:
[ +30,414603] xhci_hcd 0000:00:10.0: xHCI Host Controller [ +0,000012] xhci_hcd 0000:00:10.0: new USB bus registered, assigned bus number 2 [ +0,000159] BUG: kernel NULL pointer dereference, address: 0000000000000000 [ +0,000004] #PF: supervisor read access in kernel mode [ +0,000002] #PF: error_code(0x0000) - not-present page [ +0,000002] PGD 0 P4D 0 [ +0,000003] Oops: 0000 [#1] PREEMPT SMP [ +0,000004] CPU: 1 PID: 4270 Comm: insmod Not tainted 6.6.113 #11 [ +0,000003] Hardware name: HP HP EliteDesk 705 G3 MT/8265, BIOS P06 Ver. 02.45 07/16/2024 [ +0,000003] RIP: 0010:xhci_add_interrupter+0x25/0x130 [xhci_hcd]
[ +0,042495] xhci_hcd 0000:00:10.0: xHCI Host Controller [ +0,000012] xhci_hcd 0000:00:10.0: new USB bus registered, assigned bus number 2 [ +0,007193] xhci_hcd 0000:00:10.0: can't setup: -12 [ +0,000010] xhci_hcd 0000:00:10.0: USB bus 2 deregistered [ +0,000080] xhci_hcd 0000:00:10.0: init 0000:00:10.0 fail, -12 [ +0,000004] xhci_hcd: probe of 0000:00:10.0 failed with error -12
On Mon, Nov 03, 2025 at 09:40:36AM +0100, Michal Pecio wrote:
On Thu, 18 Sep 2025 21:08:38 +0800, Guangshuo Li wrote:
kcalloc_node() may fail. When the interrupter array allocation returns NULL, subsequent code uses xhci->interrupters (e.g. in xhci_add_interrupter() and in cleanup paths), leading to a potential NULL pointer dereference.
Check the allocation and bail out to the existing fail path to avoid the NULL dereference.
Fixes: c99b38c412343 ("xhci: add support to allocate several interrupters") Cc: stable@vger.kernel.org Signed-off-by: Guangshuo Li lgs201920130244@gmail.com
drivers/usb/host/xhci-mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index d698095fc88d..da257856e864 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -2505,7 +2505,8 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags) "Allocating primary event ring"); xhci->interrupters = kcalloc_node(xhci->max_interrupters, sizeof(*xhci->interrupters), flags, dev_to_node(dev));
- if (!xhci->interrupters)
goto fail;-- 2.43.0
Hi Greg and Mathias,
I noticed that this bug still exists in current 6.6 and 6.12 releases, what would be the sensible course of action to fix it?
The patch from Guangshuo Li is a specific fix and it applies cleanly on those branches. By simulating allocation failure, I confirmed the bug and the fix on 6.6.113, which is identical to the current 6.6.116.
Mainline added an identical check in 83d98dea48eb ("usb: xhci: add individual allocation checks in xhci_mem_init()") which is a cleanup that has a merge conflict at least with 6.6.
The conflict seems superficial and probably the cleanup would have no side effects on 6.6/6.12, but I haven't really reviewed this and maybe it would be simpler to just take the targeted fix?
A backport is always appreciated if you want a patch in stable kernels. And as this really can never be triggerd by a user, it's a low priority :)
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Guangshuo Li -
Mathias Nyman -
Michal Pecio