linux-kernel - Re: [PATCH] usb: xhci: Fix invalid pointer dereference in Etron workaround

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-prev] [day] [month] [year] [list]

Message-ID: <CAHN5xi1RgkKQ2J8tJ5+hWUksMm1F9wU6Lx9p_VgqGwCp4812GA@mail.gmail.com>
Date: Wed, 19 Mar 2025 20:56:43 +0800
From: Kuangyi Chiang <ki.chiang65@...il.com>
To: Michal Pecio <michal.pecio@...il.com>
Cc: Mathias Nyman <mathias.nyman@...el.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-usb@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: xhci: Fix invalid pointer dereference in Etron workaround

Hi,

Michal Pecio <michal.pecio@...il.com> 於 2025年3月18日 週二 上午5:29寫道：
>
> This check is performed before prepare_transfer() and prepare_ring(), so
> enqueue can already point at the final link TRB of a segment. And indeed
> it will, some 0.4% of times this code is called.
>
> Then enqueue + 1 is an invalid pointer. It will crash the kernel right
> away or load some junk which may look like a link TRB and cause the real
> link TRB to be replaced with a NOOP. This wouldn't end well.

Ah, this could happen, my bad.

>
> Use a functionally equivalent test which doesn't dereference the pointer
> and always gives correct result.
>
> Something has crashed my machine twice in recent days while playing with
> an Etron HC, and a control transfer stress test ran for confirmation has
> just crashed it again. The same test passes with this patch applied.
>
> Fixes: 5e1c67abc930 ("xhci: Fix control transfer error on Etron xHCI host")
> Cc: stable@...r.kernel.org
> Signed-off-by: Michal Pecio <michal.pecio@...il.com>
> ---
>  drivers/usb/host/xhci-ring.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 60d89cf418da..dc674bc24a89 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3786,7 +3786,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
>                  * enqueue a No Op TRB, this can prevent the Setup and Data Stage
>                  * TRB to be breaked by the Link TRB.
>                  */
> -               if (trb_is_link(ep_ring->enqueue + 1)) {
> +               if (last_trb_on_seg(ep_ring->enq_seg, ep_ring->enqueue + 1)) {
>                         field = TRB_TYPE(TRB_TR_NOOP) | ep_ring->cycle_state;
>                         queue_trb(xhci, ep_ring, false, 0, 0,
>                                         TRB_INTR_TARGET(0), field);
> --
> 2.48.1

Reviewed-by: Kuangyi Chiang <ki.chiang65@...il.com>

Thanks for the patch,
Kuangyi Chiang