linux-kernel - Re: [PATCH] usb: cdns3: remove fetched trb from cache before dequeuing

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

Message-ID: <CAL411-o4BETLPd-V_4yR6foXbES=72-P4tq-fQ_W_p0P_3ZqEw@mail.gmail.com>
Date:   Thu, 17 Nov 2022 20:00:00 +0800
From:   Peter Chen <hzpeterchen@...il.com>
To:     Pawel Laszczak <pawell@...ence.com>
Cc:     peter.chen@...nel.org, linux-usb@...r.kernel.org,
        gregkh@...uxfoundation.org, felipe.balbi@...ux.intel.com,
        rogerq@...nel.org, a-govindraju@...com,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] usb: cdns3: remove fetched trb from cache before dequeuing

On Tue, Nov 15, 2022 at 6:01 PM Pawel Laszczak <pawell@...ence.com> wrote:
>
> After doorbell DMA fetches the TRB. If during dequeuing request
> driver changes NORMAL TRB to LINK TRB but doesn't delete it from
> controller cache then controller will handle cached TRB and packet
> can be lost.
>
> The example scenario for this issue looks like:
> 1. queue request - set doorbell
> 2. dequeue request
> 3. send OUT data packet from host
> 4. Device will accept this packet which is unexpected
> 5. queue new request - set doorbell
> 6. Device lost the expected packet.
>
> By setting DFLUSH controller clears DRDY bit and stop DMA transfer.
>
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> cc: <stable@...r.kernel.org>
> Signed-off-by: Pawel Laszczak <pawell@...ence.com>
> ---
>  drivers/usb/cdns3/cdns3-gadget.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
> index 5adcb349718c..ccfaebca6faa 100644
> --- a/drivers/usb/cdns3/cdns3-gadget.c
> +++ b/drivers/usb/cdns3/cdns3-gadget.c
> @@ -2614,6 +2614,7 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
>         u8 req_on_hw_ring = 0;
>         unsigned long flags;
>         int ret = 0;
> +       int val;
>
>         if (!ep || !request || !ep->desc)
>                 return -EINVAL;
> @@ -2649,6 +2650,13 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
>
>         /* Update ring only if removed request is on pending_req_list list */
>         if (req_on_hw_ring && link_trb) {
> +               /* Stop DMA */
> +               writel(EP_CMD_DFLUSH, &priv_dev->regs->ep_cmd);
> +
> +               /* wait for DFLUSH cleared */
> +               readl_poll_timeout_atomic(&priv_dev->regs->ep_cmd, val,
> +                                         !(val & EP_CMD_DFLUSH), 1, 1000);
> +
>                 link_trb->buffer = cpu_to_le32(TRB_BUFFER(priv_ep->trb_pool_dma +
>                         ((priv_req->end_trb + 1) * TRB_SIZE)));
>                 link_trb->control = cpu_to_le32((le32_to_cpu(link_trb->control) & TRB_CYCLE) |
> @@ -2660,6 +2668,10 @@ int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
>
>         cdns3_gadget_giveback(priv_ep, priv_req, -ECONNRESET);
>
> +       req = cdns3_next_request(&priv_ep->pending_req_list);
> +       if (req)
> +               cdns3_rearm_transfer(priv_ep, 1);
> +

Why the above changes are needed?

Peter

>  not_found:
>         spin_unlock_irqrestore(&priv_dev->lock, flags);
>         return ret;
> --
> 2.25.1
>