* [PATCH] usb: xhci: Fix NULL pointer dereference as part of completion
@ 2020-05-05 6:14 Sriharsha Allenki
2020-05-05 8:24 ` Mathias Nyman
0 siblings, 1 reply; 2+ messages in thread
From: Sriharsha Allenki @ 2020-05-05 6:14 UTC (permalink / raw)
To: mathias.nyman, gregkh
Cc: linux-usb, mgautam, jackp, Sriharsha Allenki, stable
On platforms with IOMMU enabled, multiple SGs can be
coalesced into one by the IOMMU driver. In that case
the SG list processing as part of the completion of
a urb on a bulk endpoint can result into a NULL pointer
dereference with the below stack dump.
<6> Unable to handle kernel NULL pointer dereference at virtual address 0000000c
<6> pgd = c0004000
<6> [0000000c] *pgd=00000000
<6> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
<2> PC is at xhci_queue_bulk_tx+0x454/0x80c
<2> LR is at xhci_queue_bulk_tx+0x44c/0x80c
<2> pc : [<c08907c4>] lr : [<c08907bc>] psr: 000000d3
<2> sp : ca337c80 ip : 00000000 fp : ffffffff
<2> r10: 00000000 r9 : 50037000 r8 : 00004000
<2> r7 : 00000000 r6 : 00004000 r5 : 00000000 r4 : 00000000
<2> r3 : 00000000 r2 : 00000082 r1 : c2c1a200 r0 : 00000000
<2> Flags: nzcv IRQs off FIQs off Mode SVC_32 ISA ARM Segment none
<2> Control: 10c0383d Table: b412c06a DAC: 00000051
<6> Process usb-storage (pid: 5961, stack limit = 0xca336210)
<snip>
<2> [<c08907c4>] (xhci_queue_bulk_tx)
<2> [<c0881b3c>] (xhci_urb_enqueue)
<2> [<c0831068>] (usb_hcd_submit_urb)
<2> [<c08350b4>] (usb_sg_wait)
<2> [<c089f384>] (usb_stor_bulk_transfer_sglist)
<2> [<c089f2c0>] (usb_stor_bulk_srb)
<2> [<c089fe38>] (usb_stor_Bulk_transport)
<2> [<c089f468>] (usb_stor_invoke_transport)
<2> [<c08a11b4>] (usb_stor_control_thread)
<2> [<c014a534>] (kthread)
The above NULL pointer dereference is the result of block_len and
the sent_len set to zero after the first SG of the list when IOMMU
driver is enabled. Because of this the loop of processing the SGs
has run more than num_sgs which resulted in a sg_next on the
last SG of the list which has SG_END set.
Fix this by check for the sg before any attributes of the sg are
accessed.
Fixes: f9c589e142d04 ("xhci: TD-fragment, align the unsplittable case with a bounce buffer")
Cc: stable@vger.kernel.org
Signed-off-by: Sriharsha Allenki <sallenki@codeaurora.org>
---
drivers/usb/host/xhci-ring.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a78787bb5133..18141b38f7bf 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3399,8 +3399,8 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
/* New sg entry */
--num_sgs;
sent_len -= block_len;
- if (num_sgs != 0) {
- sg = sg_next(sg);
+ sg = sg_next(sg);
+ if (num_sgs != 0 && sg) {
block_len = sg_dma_len(sg);
addr = (u64) sg_dma_address(sg);
addr += sent_len;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] usb: xhci: Fix NULL pointer dereference as part of completion
2020-05-05 6:14 [PATCH] usb: xhci: Fix NULL pointer dereference as part of completion Sriharsha Allenki
@ 2020-05-05 8:24 ` Mathias Nyman
0 siblings, 0 replies; 2+ messages in thread
From: Mathias Nyman @ 2020-05-05 8:24 UTC (permalink / raw)
To: Sriharsha Allenki, mathias.nyman, gregkh
Cc: linux-usb, mgautam, jackp, stable
On 5.5.2020 9.14, Sriharsha Allenki wrote:
> On platforms with IOMMU enabled, multiple SGs can be
> coalesced into one by the IOMMU driver. In that case
> the SG list processing as part of the completion of
> a urb on a bulk endpoint can result into a NULL pointer
> dereference with the below stack dump.
>
> <6> Unable to handle kernel NULL pointer dereference at virtual address 0000000c
> <6> pgd = c0004000
> <6> [0000000c] *pgd=00000000
> <6> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> <2> PC is at xhci_queue_bulk_tx+0x454/0x80c
> <2> LR is at xhci_queue_bulk_tx+0x44c/0x80c
> <2> pc : [<c08907c4>] lr : [<c08907bc>] psr: 000000d3
> <2> sp : ca337c80 ip : 00000000 fp : ffffffff
> <2> r10: 00000000 r9 : 50037000 r8 : 00004000
> <2> r7 : 00000000 r6 : 00004000 r5 : 00000000 r4 : 00000000
> <2> r3 : 00000000 r2 : 00000082 r1 : c2c1a200 r0 : 00000000
> <2> Flags: nzcv IRQs off FIQs off Mode SVC_32 ISA ARM Segment none
> <2> Control: 10c0383d Table: b412c06a DAC: 00000051
> <6> Process usb-storage (pid: 5961, stack limit = 0xca336210)
> <snip>
> <2> [<c08907c4>] (xhci_queue_bulk_tx)
> <2> [<c0881b3c>] (xhci_urb_enqueue)
> <2> [<c0831068>] (usb_hcd_submit_urb)
> <2> [<c08350b4>] (usb_sg_wait)
> <2> [<c089f384>] (usb_stor_bulk_transfer_sglist)
> <2> [<c089f2c0>] (usb_stor_bulk_srb)
> <2> [<c089fe38>] (usb_stor_Bulk_transport)
> <2> [<c089f468>] (usb_stor_invoke_transport)
> <2> [<c08a11b4>] (usb_stor_control_thread)
> <2> [<c014a534>] (kthread)
>
> The above NULL pointer dereference is the result of block_len and
> the sent_len set to zero after the first SG of the list when IOMMU
> driver is enabled. Because of this the loop of processing the SGs
> has run more than num_sgs which resulted in a sg_next on the
> last SG of the list which has SG_END set.
>
> Fix this by check for the sg before any attributes of the sg are
> accessed.
>
> Fixes: f9c589e142d04 ("xhci: TD-fragment, align the unsplittable case with a bounce buffer")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sriharsha Allenki <sallenki@codeaurora.org>
> ---
> drivers/usb/host/xhci-ring.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index a78787bb5133..18141b38f7bf 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -3399,8 +3399,8 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
> /* New sg entry */
> --num_sgs;
> sent_len -= block_len;
> - if (num_sgs != 0) {
> - sg = sg_next(sg);
> + sg = sg_next(sg);
> + if (num_sgs != 0 && sg) {
> block_len = sg_dma_len(sg);
> addr = (u64) sg_dma_address(sg);
> addr += sent_len;
>
Nice catch, thanks.
Adding to queue.
-Mathias
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-05-05 8:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 6:14 [PATCH] usb: xhci: Fix NULL pointer dereference as part of completion Sriharsha Allenki
2020-05-05 8:24 ` Mathias Nyman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.