[PATCH] usb: gadget: udc-xilinx: Fix wild pointer dereference on empty EP0 queue

[PATCH] usb: gadget: udc-xilinx: Fix wild pointer dereference on empty EP0 queue

[Li Ximo]

From: Wang Haoran <haoranwangsec@gmail.com>

list_first_entry() does not return NULL when the list is empty; it
returns the result of container_of() on the list head, i.e. a non-NULL
wild pointer pointing into the enclosing struct xusb_ep rather than to
a valid request. This was found by code audit.

xudc_ep0_out() and xudc_ep0_in() are reached from the endpoint 0
interrupt handler xudc_ctrl_ep_handler() purely based on the hardware
FIFO status bits, without checking whether ep0->queue actually holds a
request. The queue can be empty at that point: the gadget driver's
setup() callback may not queue a request for every control transfer
stage, and xudc_done() removes the request once it completes, after
which bus timing can raise another FIFO ready/free interrupt. The
unconditional list_first_entry() then yields a wild pointer, and the
subsequent accesses to req->usb_req.actual, req->usb_req.buf and
req->usb_req.length read or write unrelated kernel memory, leading to
memory corruption or a crash. On real hardware this is triggerable by
an external USB host controlling the token timing. Note that the
enqueue side __xudc_ep0_queue() already guards the same queue with
list_empty(), which shows the queue can legitimately be empty; the
dequeue side was simply missing the symmetric check.

Switch both functions to list_first_entry_or_null() and return early
when no request is queued, mirroring the existing list_empty() guard
on the enqueue side.

Reported-by: Wang Haoran <haoranwangsec@gmail.com>
Reported-by: Li Ximo <lixicaicai@gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
---
 drivers/usb/gadget/udc/udc-xilinx.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
index bef06fe75..032d9f04f 100644
--- a/drivers/usb/gadget/udc/udc-xilinx.c
+++ b/drivers/usb/gadget/udc/udc-xilinx.c
@@ -1822,7 +1822,9 @@ static void xudc_ep0_out(struct xusb_udc *udc)
 	unsigned int bytes_to_rx;
 	void *buffer;
 
-	req = list_first_entry(&ep0->queue, struct xusb_req, queue);
+	req = list_first_entry_or_null(&ep0->queue, struct xusb_req, queue);
+	if (!req)
+		return;
 
 	switch (udc->setupseqrx) {
 	case STATUS_PHASE:
@@ -1875,7 +1877,10 @@ static void xudc_ep0_in(struct xusb_udc *udc)
 	u8 *ep0rambase;
 	u8 test_mode = le16_to_cpu(udc->setup.wIndex) >> 8;
 
-	req = list_first_entry(&ep0->queue, struct xusb_req, queue);
+	req = list_first_entry_or_null(&ep0->queue, struct xusb_req, queue);
+	if (!req)
+		return;
+
 	bytes_to_tx = req->usb_req.length - req->usb_req.actual;
 
 	switch (udc->setupseqtx) {
-- 
2.30.2

Re: [PATCH] usb: gadget: udc-xilinx: Fix wild pointer dereference on empty EP0 queue

[Aldo Ariel Panzardo]

Reviewed-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

Re: [PATCH] usb: gadget: udc-xilinx: Fix wild pointer dereference on empty EP0 queue

[S M, Suhan]

On 10/23/2026 1:01 PM, Li Ximo wrote:
> From: Wang Haoran <haoranwangsec@gmail.com>
> 
> list_first_entry() does not return NULL when the list is empty; it
> returns the result of container_of() on the list head, i.e. a non-NULL
> wild pointer pointing into the enclosing struct xusb_ep rather than to
> a valid request. This was found by code audit.
> 
> xudc_ep0_out() and xudc_ep0_in() are reached from the endpoint 0
> interrupt handler xudc_ctrl_ep_handler() purely based on the hardware
> FIFO status bits, without checking whether ep0->queue actually holds a
> request. The queue can be empty at that point: the gadget driver's
> setup() callback may not queue a request for every control transfer
> stage, and xudc_done() removes the request once it completes, after
> which bus timing can raise another FIFO ready/free interrupt. The
The claim that setup() may not queue a request for every control
transfer stage does not explain how xudc_ep0_in() or xudc_ep0_out()
run with an empty list. Those functions are reached from Buff Free /
Buff Rdy, which complete an EP0 buffer that __xudc_ep0_queue()
already armed with BUFFREADY.

If setup() never calls usb_ep_queue(), that write never happens. The
SIE NAKs host IN/OUT, and this code is not reachable for that stage.

So "bus timing can raise another FIFO ready/free interrupt" is only
true if software already armed a buffer. Timing of a bare token does
not create that IRQ.
> unconditional list_first_entry() then yields a wild pointer, and the
> subsequent accesses to req->usb_req.actual, req->usb_req.buf and
> req->usb_req.length read or write unrelated kernel memory, leading to
> memory corruption or a crash. On real hardware this is triggerable by
> an external USB host controlling the token timing. Note that the
> enqueue side __xudc_ep0_queue() already guards the same queue with
> list_empty(), which shows the queue can legitimately be empty; the
> dequeue side was simply missing the symmetric check.
> 
> Switch both functions to list_first_entry_or_null() and return early
> when no request is queued, mirroring the existing list_empty() guard
> on the enqueue side.
> 
> Reported-by: Wang Haoran <haoranwangsec@gmail.com>
> Reported-by: Li Ximo <lixicaicai@gmail.com>
> Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
> ---
>   drivers/usb/gadget/udc/udc-xilinx.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
> index bef06fe75..032d9f04f 100644
> --- a/drivers/usb/gadget/udc/udc-xilinx.c
> +++ b/drivers/usb/gadget/udc/udc-xilinx.c
> @@ -1822,7 +1822,9 @@ static void xudc_ep0_out(struct xusb_udc *udc)
>   	unsigned int bytes_to_rx;
>   	void *buffer;
>   
> -	req = list_first_entry(&ep0->queue, struct xusb_req, queue);
> +	req = list_first_entry_or_null(&ep0->queue, struct xusb_req, queue);
> +	if (!req)
> +		return;
>   
>   	switch (udc->setupseqrx) {
>   	case STATUS_PHASE:
> @@ -1875,7 +1877,10 @@ static void xudc_ep0_in(struct xusb_udc *udc)
>   	u8 *ep0rambase;
>   	u8 test_mode = le16_to_cpu(udc->setup.wIndex) >> 8;
>   
> -	req = list_first_entry(&ep0->queue, struct xusb_req, queue);
> +	req = list_first_entry_or_null(&ep0->queue, struct xusb_req, queue);
> +	if (!req)
> +		return;
> +
>   	bytes_to_tx = req->usb_req.length - req->usb_req.actual;
>   
>   	switch (udc->setupseqtx) {
This check looks nice-to-have, as the non-control endpoint handler
already performs the same check. Please refactor the commit description
accordingly and do add the fixes tag .