* Re: Patch "xhci: fix possible null pointer deref during xhci urb enqueue" has been added to the 6.7-stab
[not found] <20240223174501.3260504-1-sashal () kernel ! org>
@ 2024-02-25 17:31 ` Pascal Ernster
2024-02-26 6:01 ` Greg Kroah-Hartman
0 siblings, 1 reply; 2+ messages in thread
From: Pascal Ernster @ 2024-02-25 17:31 UTC (permalink / raw)
To: Sasha Levin, stable; +Cc: Greg Kroah-Hartman
[2024-02-23 18:45] Sasha Levin:
> This is a note to let you know that I've just added the patch titled
>
> xhci: fix possible null pointer deref during xhci urb enqueue
>
> to the 6.7-stable tree which can be found at:
> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>
> The filename of the patch is:
> xhci-fix-possible-null-pointer-deref-during-xhci-urb.patch
> and it can be found in the queue-6.7 subdirectory.
>
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
>
>
>
> commit fb9100c2c6b7b172650ba25283cc4cf9af1d082c
> Author: Mathias Nyman <mathias.nyman@linux.intel.com>
> Date: Fri Dec 1 17:06:47 2023 +0200
>
> xhci: fix possible null pointer deref during xhci urb enqueue
>
> [ Upstream commit e2e2aacf042f52854c92775b7800ba668e0bdfe4 ]
>
> There is a short gap between urb being submitted and actually added to the
> endpoint queue (linked). If the device is disconnected during this time
> then usb core is not yet aware of the pending urb, and device may be freed
> just before xhci_urq_enqueue() continues, dereferencing the freed device.
>
> Freeing the device is protected by the xhci spinlock, so make sure we take
> and keep the lock while checking that device exists, dereference it, and
> add the urb to the queue.
>
> Remove the unnecessary URB check, usb core checks it before calling
> xhci_urb_enqueue()
>
> Suggested-by: Kuen-Han Tsai <khtsai@google.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> Link: https://lore.kernel.org/r/20231201150647.1307406-20-mathias.nyman@linux.intel.com
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 884b0898d9c95..ddb686301af5d 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1522,24 +1522,7 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
> struct urb_priv *urb_priv;
> int num_tds;
>
> - if (!urb)
> - return -EINVAL;
> - ret = xhci_check_args(hcd, urb->dev, urb->ep,
> - true, true, __func__);
> - if (ret <= 0)
> - return ret ? ret : -EINVAL;
> -
> - slot_id = urb->dev->slot_id;
> ep_index = xhci_get_endpoint_index(&urb->ep->desc);
> - ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state;
> -
> - if (!HCD_HW_ACCESSIBLE(hcd))
> - return -ESHUTDOWN;
> -
> - if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) {
> - xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n");
> - return -ENODEV;
> - }
>
> if (usb_endpoint_xfer_isoc(&urb->ep->desc))
> num_tds = urb->number_of_packets;
> @@ -1578,12 +1561,35 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
>
> spin_lock_irqsave(&xhci->lock, flags);
>
> + ret = xhci_check_args(hcd, urb->dev, urb->ep,
> + true, true, __func__);
> + if (ret <= 0) {
> + ret = ret ? ret : -EINVAL;
> + goto free_priv;
> + }
> +
> + slot_id = urb->dev->slot_id;
> +
> + if (!HCD_HW_ACCESSIBLE(hcd)) {
> + ret = -ESHUTDOWN;
> + goto free_priv;
> + }
> +
> + if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) {
> + xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n");
> + ret = -ENODEV;
> + goto free_priv;
> + }
> +
> if (xhci->xhc_state & XHCI_STATE_DYING) {
> xhci_dbg(xhci, "Ep 0x%x: URB %p submitted for non-responsive xHCI host.\n",
> urb->ep->desc.bEndpointAddress, urb);
> ret = -ESHUTDOWN;
> goto free_priv;
> }
> +
> + ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state;
> +
> if (*ep_state & (EP_GETTING_STREAMS | EP_GETTING_NO_STREAMS)) {
> xhci_warn(xhci, "WARN: Can't enqueue URB, ep in streams transition state %x\n",
> *ep_state);
Hi, this patch is causing my laptop (Dell Precision 7530) to crash
during early boot with a kernel 6.7.6 with all the patches from your
current stable-queue applied on top
(https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/queue-6.7?id=8294c8ea0d96dc8271e87053f7b6731ee5b986ca).
Booting with "module_blacklist=xhci_pci,xhci_pci_renesas" stops the
crashes. This patch was already thrown out a few weeks ago because it
was causing problems:
https://lore.kernel.org/stable/2024020331-confetti-ducking-8afb@gregkh/
Regards
Pascal
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Patch "xhci: fix possible null pointer deref during xhci urb enqueue" has been added to the 6.7-stab
2024-02-25 17:31 ` Patch "xhci: fix possible null pointer deref during xhci urb enqueue" has been added to the 6.7-stab Pascal Ernster
@ 2024-02-26 6:01 ` Greg Kroah-Hartman
0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2024-02-26 6:01 UTC (permalink / raw)
To: Pascal Ernster; +Cc: Sasha Levin, stable
On Sun, Feb 25, 2024 at 06:31:04PM +0100, Pascal Ernster wrote:
> [2024-02-23 18:45] Sasha Levin:
> > This is a note to let you know that I've just added the patch titled
> >
> > xhci: fix possible null pointer deref during xhci urb enqueue
> >
> > to the 6.7-stable tree which can be found at:
> > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> > The filename of the patch is:
> > xhci-fix-possible-null-pointer-deref-during-xhci-urb.patch
> > and it can be found in the queue-6.7 subdirectory.
> >
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> >
> >
> >
> > commit fb9100c2c6b7b172650ba25283cc4cf9af1d082c
> > Author: Mathias Nyman <mathias.nyman@linux.intel.com>
> > Date: Fri Dec 1 17:06:47 2023 +0200
> >
> > xhci: fix possible null pointer deref during xhci urb enqueue
> > [ Upstream commit e2e2aacf042f52854c92775b7800ba668e0bdfe4 ]
> > There is a short gap between urb being submitted and actually added to the
> > endpoint queue (linked). If the device is disconnected during this time
> > then usb core is not yet aware of the pending urb, and device may be freed
> > just before xhci_urq_enqueue() continues, dereferencing the freed device.
> > Freeing the device is protected by the xhci spinlock, so make sure we take
> > and keep the lock while checking that device exists, dereference it, and
> > add the urb to the queue.
> > Remove the unnecessary URB check, usb core checks it before calling
> > xhci_urb_enqueue()
> > Suggested-by: Kuen-Han Tsai <khtsai@google.com>
> > Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > Link: https://lore.kernel.org/r/20231201150647.1307406-20-mathias.nyman@linux.intel.com
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Sasha Levin <sashal@kernel.org>
> >
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 884b0898d9c95..ddb686301af5d 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -1522,24 +1522,7 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
> > struct urb_priv *urb_priv;
> > int num_tds;
> > - if (!urb)
> > - return -EINVAL;
> > - ret = xhci_check_args(hcd, urb->dev, urb->ep,
> > - true, true, __func__);
> > - if (ret <= 0)
> > - return ret ? ret : -EINVAL;
> > -
> > - slot_id = urb->dev->slot_id;
> > ep_index = xhci_get_endpoint_index(&urb->ep->desc);
> > - ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state;
> > -
> > - if (!HCD_HW_ACCESSIBLE(hcd))
> > - return -ESHUTDOWN;
> > -
> > - if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) {
> > - xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n");
> > - return -ENODEV;
> > - }
> > if (usb_endpoint_xfer_isoc(&urb->ep->desc))
> > num_tds = urb->number_of_packets;
> > @@ -1578,12 +1561,35 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
> > spin_lock_irqsave(&xhci->lock, flags);
> > + ret = xhci_check_args(hcd, urb->dev, urb->ep,
> > + true, true, __func__);
> > + if (ret <= 0) {
> > + ret = ret ? ret : -EINVAL;
> > + goto free_priv;
> > + }
> > +
> > + slot_id = urb->dev->slot_id;
> > +
> > + if (!HCD_HW_ACCESSIBLE(hcd)) {
> > + ret = -ESHUTDOWN;
> > + goto free_priv;
> > + }
> > +
> > + if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) {
> > + xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n");
> > + ret = -ENODEV;
> > + goto free_priv;
> > + }
> > +
> > if (xhci->xhc_state & XHCI_STATE_DYING) {
> > xhci_dbg(xhci, "Ep 0x%x: URB %p submitted for non-responsive xHCI host.\n",
> > urb->ep->desc.bEndpointAddress, urb);
> > ret = -ESHUTDOWN;
> > goto free_priv;
> > }
> > +
> > + ep_state = &xhci->devs[slot_id]->eps[ep_index].ep_state;
> > +
> > if (*ep_state & (EP_GETTING_STREAMS | EP_GETTING_NO_STREAMS)) {
> > xhci_warn(xhci, "WARN: Can't enqueue URB, ep in streams transition state %x\n",
> > *ep_state);
>
>
> Hi, this patch is causing my laptop (Dell Precision 7530) to crash during
> early boot with a kernel 6.7.6 with all the patches from your current
> stable-queue applied on top (https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/queue-6.7?id=8294c8ea0d96dc8271e87053f7b6731ee5b986ca).
>
> Booting with "module_blacklist=xhci_pci,xhci_pci_renesas" stops the crashes.
> This patch was already thrown out a few weeks ago because it was causing
> problems:
>
> https://lore.kernel.org/stable/2024020331-confetti-ducking-8afb@gregkh/
Yeah, thanks, this one needs to be dropped as it depended on previous
patches to work properly, I'll go drop them now, thanks!
greg k-h
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-02-26 6:01 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240223174501.3260504-1-sashal () kernel ! org>
2024-02-25 17:31 ` Patch "xhci: fix possible null pointer deref during xhci urb enqueue" has been added to the 6.7-stab Pascal Ernster
2024-02-26 6:01 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox