2020-07-12 10:30:13

by Jon Hunter

[permalink] [raw]
Subject: [PATCH 1/2] usb: tegra: Fix allocation for the FPCI context

Commit 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB
context save/restore") is using the IPFS 'num_offsets' value when
allocating memory for FPCI context instead of the FPCI 'num_offsets'.
We have not observed any specific issues because of this, but could
cause too much memory or too little memory to be allocated. Fix this
by using the FPCI 'num_offsets' for allocating the FPCI memory for
storing the FPCI state.

Cc: [email protected]

Fixes: 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB context save/restore")

Signed-off-by: Jon Hunter <[email protected]>
---
drivers/usb/host/xhci-tegra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 9ce28ab47f4b..014d79334f50 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -856,7 +856,7 @@ static int tegra_xusb_init_context(struct tegra_xusb *tegra)
if (!tegra->context.ipfs)
return -ENOMEM;

- tegra->context.fpci = devm_kcalloc(tegra->dev, soc->ipfs.num_offsets,
+ tegra->context.fpci = devm_kcalloc(tegra->dev, soc->fpci.num_offsets,
sizeof(u32), GFP_KERNEL);
if (!tegra->context.fpci)
return -ENOMEM;
--
2.17.1


2020-07-14 09:20:46

by Thierry Reding

[permalink] [raw]
Subject: Re: [PATCH 1/2] usb: tegra: Fix allocation for the FPCI context

On Sun, Jul 12, 2020 at 11:28:36AM +0100, Jon Hunter wrote:
> Commit 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB
> context save/restore") is using the IPFS 'num_offsets' value when
> allocating memory for FPCI context instead of the FPCI 'num_offsets'.
> We have not observed any specific issues because of this, but could
> cause too much memory or too little memory to be allocated. Fix this
> by using the FPCI 'num_offsets' for allocating the FPCI memory for
> storing the FPCI state.
>
> Cc: [email protected]
>
> Fixes: 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB context save/restore")
>
> Signed-off-by: Jon Hunter <[email protected]>
> ---
> drivers/usb/host/xhci-tegra.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

Good catch!

Acked-by: Thierry Reding <[email protected]>


Attachments:
(No filename) (860.00 B)
signature.asc (849.00 B)
Download all attachments

2020-07-15 12:18:48

by Jon Hunter

[permalink] [raw]
Subject: [PATCH V2] usb: tegra: Fix allocation for the FPCI context

Commit 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB
context save/restore") is using the IPFS 'num_offsets' value when
allocating memory for FPCI context instead of the FPCI 'num_offsets'.

After commit cad064f1bd52 ("devres: handle zero size in devm_kmalloc()")
was added system suspend started failing on Tegra186. The kernel log
showed that the Tegra XHCI driver was crashing on entry to suspend when
attempting the save the USB context. On Tegra186, the IPFS context has a
zero length but the FPCI content has a non-zero length, and because of
the bug in the Tegra XHCI driver we are incorrectly allocating a zero
length array for the FPCI context. The crash seen on entering suspend
when we attempt to save the FPCI context and following commit
cad064f1bd52 ("devres: handle zero size in devm_kmalloc()") this now
causes a NULL pointer deference when we access the memory. Fix this by
correcting the amount of memory we are allocating for FPCI contexts.

Cc: [email protected]

Fixes: 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB context save/restore")

Signed-off-by: Jon Hunter <[email protected]>
Acked-by: Thierry Reding <[email protected]>
---

Changes since V1:
- Corrected commit message
- Added Thierry's ACK

drivers/usb/host/xhci-tegra.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 9ce28ab47f4b..014d79334f50 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -856,7 +856,7 @@ static int tegra_xusb_init_context(struct tegra_xusb *tegra)
if (!tegra->context.ipfs)
return -ENOMEM;

- tegra->context.fpci = devm_kcalloc(tegra->dev, soc->ipfs.num_offsets,
+ tegra->context.fpci = devm_kcalloc(tegra->dev, soc->fpci.num_offsets,
sizeof(u32), GFP_KERNEL);
if (!tegra->context.fpci)
return -ENOMEM;
--
2.17.1

2020-07-23 11:20:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH V2] usb: tegra: Fix allocation for the FPCI context

On Wed, Jul 15, 2020 at 12:38:42PM +0100, Jon Hunter wrote:
> Commit 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB
> context save/restore") is using the IPFS 'num_offsets' value when
> allocating memory for FPCI context instead of the FPCI 'num_offsets'.
>
> After commit cad064f1bd52 ("devres: handle zero size in devm_kmalloc()")
> was added system suspend started failing on Tegra186. The kernel log
> showed that the Tegra XHCI driver was crashing on entry to suspend when
> attempting the save the USB context. On Tegra186, the IPFS context has a
> zero length but the FPCI content has a non-zero length, and because of
> the bug in the Tegra XHCI driver we are incorrectly allocating a zero
> length array for the FPCI context. The crash seen on entering suspend
> when we attempt to save the FPCI context and following commit
> cad064f1bd52 ("devres: handle zero size in devm_kmalloc()") this now
> causes a NULL pointer deference when we access the memory. Fix this by
> correcting the amount of memory we are allocating for FPCI contexts.
>
> Cc: [email protected]
>
> Fixes: 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB context save/restore")
>
> Signed-off-by: Jon Hunter <[email protected]>
> Acked-by: Thierry Reding <[email protected]>
> ---
>
> Changes since V1:
> - Corrected commit message
> - Added Thierry's ACK
>
> drivers/usb/host/xhci-tegra.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

No cc: to linux-usb@vger? :(

I'll go queue this up, but I would have caught it sooner if you had done
so...

thanks,

greg k-h

2020-07-29 10:08:46

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH V2] usb: tegra: Fix allocation for the FPCI context


On 23/07/2020 12:19, Greg Kroah-Hartman wrote:
> On Wed, Jul 15, 2020 at 12:38:42PM +0100, Jon Hunter wrote:
>> Commit 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB
>> context save/restore") is using the IPFS 'num_offsets' value when
>> allocating memory for FPCI context instead of the FPCI 'num_offsets'.
>>
>> After commit cad064f1bd52 ("devres: handle zero size in devm_kmalloc()")
>> was added system suspend started failing on Tegra186. The kernel log
>> showed that the Tegra XHCI driver was crashing on entry to suspend when
>> attempting the save the USB context. On Tegra186, the IPFS context has a
>> zero length but the FPCI content has a non-zero length, and because of
>> the bug in the Tegra XHCI driver we are incorrectly allocating a zero
>> length array for the FPCI context. The crash seen on entering suspend
>> when we attempt to save the FPCI context and following commit
>> cad064f1bd52 ("devres: handle zero size in devm_kmalloc()") this now
>> causes a NULL pointer deference when we access the memory. Fix this by
>> correcting the amount of memory we are allocating for FPCI contexts.
>>
>> Cc: [email protected]
>>
>> Fixes: 5c4e8d3781bc ("usb: host: xhci-tegra: Add support for XUSB context save/restore")
>>
>> Signed-off-by: Jon Hunter <[email protected]>
>> Acked-by: Thierry Reding <[email protected]>
>> ---
>>
>> Changes since V1:
>> - Corrected commit message
>> - Added Thierry's ACK
>>
>> drivers/usb/host/xhci-tegra.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> No cc: to linux-usb@vger? :(
>
> I'll go queue this up, but I would have caught it sooner if you had done
> so...

Sorry about that. Thanks for queuing up!
Jon

--
nvpublic