| 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
| ||
|
Date: Fri, 11 Oct 2013 10:28:26 -0700
From: Sarah Sharp <sarah.a.sharp@...ux.intel.com>
To: xiao jin <jin.xiao@...el.com>
Cc: gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org,
Mathias Nyman <mathias.nyman@...ux.intel.com>,
stable@...r.kernel.org
Subject: Re: [PATCH] xhci: Ensure a command structure points to the correct
trb on the command ring
On Fri, Oct 11, 2013 at 10:25:23AM -0700, Sarah Sharp wrote:
> Hi Xiao,
>
> I think you did something odd when you tried to send me the latest
> revision of your patch "xhci: correct the usage of
> USB_CTRL_SET_TIMEOUT". You sent this patch instead. On the plus side,
> it looks like git-send-email works.
>
> Can you try again?
Ah, nevermind, I saw the v2 patch you sent later.
Sarah
> On Fri, Oct 11, 2013 at 08:47:54AM +0800, xiao jin wrote:
> > From: Mathias Nyman <mathias.nyman@...ux.intel.com>
> >
> > If a command on the command ring needs to be cancelled before it is handled
> > it can be turned to a no-op operation when the ring is stopped.
> > We want to store the command ring enqueue pointer in the command structure
> > when the command in enqueued for the cancellation case.
> >
> > Some commands used to store the command ring dequeue pointers instead of enqueue
> > (these often worked because enqueue happends to equal dequeue quite often)
> >
> > Other commands correctly used the enqueue pointer but did not check if it pointed
> > to a valid trb or a link trb, this caused for example stop endpoint command to timeout in
> > xhci_stop_device() in about 2% of suspend/resume cases.
> >
> > This should also solve some weird behavior happening in command cancellation cases.
> >
> > This patch is based on a patch submitted by Sarah Sharp to linux-usb, but
> > then forgotten:
> > http://marc.info/?l=linux-usb&m=136269803207465&w=2
> >
> > This patch should be backported to kernels as old as 3.7, that contain
> > the commit b92cc66c047ff7cf587b318fe377061a353c120f "xHCI: add aborting
> > command ring function"
> >
> > Signed-off-by: Mathias Nyman <mathias.nyman@...ux.intel.com>
> > Signed-off-by: Sarah Sharp <sarah.a.sharp@...ux.intel.com>
> > Cc: stable@...r.kernel.org
> > ---
> > drivers/usb/host/xhci-hub.c | 2 +-
> > drivers/usb/host/xhci-ring.c | 10 ++++++++++
> > drivers/usb/host/xhci.c | 25 +++++--------------------
> > drivers/usb/host/xhci.h | 1 +
> > 4 files changed, 17 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> > index fae697e..ccf0a06 100644
> > --- a/drivers/usb/host/xhci-hub.c
> > +++ b/drivers/usb/host/xhci-hub.c
> > @@ -287,7 +287,7 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int slot_id, int suspend)
> > if (virt_dev->eps[i].ring && virt_dev->eps[i].ring->dequeue)
> > xhci_queue_stop_endpoint(xhci, slot_id, i, suspend);
> > }
> > - cmd->command_trb = xhci->cmd_ring->enqueue;
> > + cmd->command_trb = xhci_find_next_enqueue(xhci->cmd_ring);
> > list_add_tail(&cmd->cmd_list, &virt_dev->cmd_list);
> > xhci_queue_stop_endpoint(xhci, slot_id, 0, suspend);
> > xhci_ring_cmd_db(xhci);
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index aaa2906..9ac9672 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -123,6 +123,16 @@ static int enqueue_is_link_trb(struct xhci_ring *ring)
> > return TRB_TYPE_LINK_LE32(link->control);
> > }
> >
> > +union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring)
> > +{
> > + /* Enqueue pointer can be left pointing to the link TRB,
> > + * we must handle that
> > + */
> > + if (TRB_TYPE_LINK_LE32(ring->enqueue->link.control))
> > + return ring->enq_seg->next->trbs;
> > + return ring->enqueue;
> > +}
> > +
> > /* Updates trb to point to the next TRB in the ring, and updates seg if the next
> > * TRB is in a new segment. This does not skip over link TRBs, and it does not
> > * effect the ring dequeue or enqueue pointers.
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 49b6edb..1e36dbb 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -2598,15 +2598,7 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci,
> > if (command) {
> > cmd_completion = command->completion;
> > cmd_status = &command->status;
> > - command->command_trb = xhci->cmd_ring->enqueue;
> > -
> > - /* Enqueue pointer can be left pointing to the link TRB,
> > - * we must handle that
> > - */
> > - if (TRB_TYPE_LINK_LE32(command->command_trb->link.control))
> > - command->command_trb =
> > - xhci->cmd_ring->enq_seg->next->trbs;
> > -
> > + command->command_trb = xhci_find_next_enqueue(xhci->cmd_ring);
> > list_add_tail(&command->cmd_list, &virt_dev->cmd_list);
> > } else {
> > cmd_completion = &virt_dev->cmd_completion;
> > @@ -2614,7 +2606,7 @@ static int xhci_configure_endpoint(struct xhci_hcd *xhci,
> > }
> > init_completion(cmd_completion);
> >
> > - cmd_trb = xhci->cmd_ring->dequeue;
> > + cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring);
> > if (!ctx_change)
> > ret = xhci_queue_configure_endpoint(xhci, in_ctx->dma,
> > udev->slot_id, must_succeed);
> > @@ -3439,14 +3431,7 @@ int xhci_discover_or_reset_device(struct usb_hcd *hcd, struct usb_device *udev)
> >
> > /* Attempt to submit the Reset Device command to the command ring */
> > spin_lock_irqsave(&xhci->lock, flags);
> > - reset_device_cmd->command_trb = xhci->cmd_ring->enqueue;
> > -
> > - /* Enqueue pointer can be left pointing to the link TRB,
> > - * we must handle that
> > - */
> > - if (TRB_TYPE_LINK_LE32(reset_device_cmd->command_trb->link.control))
> > - reset_device_cmd->command_trb =
> > - xhci->cmd_ring->enq_seg->next->trbs;
> > + reset_device_cmd->command_trb = xhci_find_next_enqueue(xhci->cmd_ring);
> >
> > list_add_tail(&reset_device_cmd->cmd_list, &virt_dev->cmd_list);
> > ret = xhci_queue_reset_device(xhci, slot_id);
> > @@ -3650,7 +3635,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
> > union xhci_trb *cmd_trb;
> >
> > spin_lock_irqsave(&xhci->lock, flags);
> > - cmd_trb = xhci->cmd_ring->dequeue;
> > + cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring);
> > ret = xhci_queue_slot_control(xhci, TRB_ENABLE_SLOT, 0);
> > if (ret) {
> > spin_unlock_irqrestore(&xhci->lock, flags);
> > @@ -3785,7 +3770,7 @@ int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev)
> > slot_ctx->dev_info >> 27);
> >
> > spin_lock_irqsave(&xhci->lock, flags);
> > - cmd_trb = xhci->cmd_ring->dequeue;
> > + cmd_trb = xhci_find_next_enqueue(xhci->cmd_ring);
> > ret = xhci_queue_address_device(xhci, virt_dev->in_ctx->dma,
> > udev->slot_id);
> > if (ret) {
> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> > index 46aa148..f3e1020 100644
> > --- a/drivers/usb/host/xhci.h
> > +++ b/drivers/usb/host/xhci.h
> > @@ -1840,6 +1840,7 @@ int xhci_cancel_cmd(struct xhci_hcd *xhci, struct xhci_command *command,
> > union xhci_trb *cmd_trb);
> > void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, unsigned int slot_id,
> > unsigned int ep_index, unsigned int stream_id);
> > +union xhci_trb *xhci_find_next_enqueue(struct xhci_ring *ring);
> >
> > /* xHCI roothub code */
> > void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
> > --
> > 1.7.1
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists