* [PATCH] phy: tegra: xusb: Fix dangling pointer on probe failure
@ 2020-10-13 9:58 Marc Zyngier
2020-10-28 16:12 ` Vinod Koul
2020-10-28 16:45 ` Thierry Reding
0 siblings, 2 replies; 3+ messages in thread
From: Marc Zyngier @ 2020-10-13 9:58 UTC (permalink / raw)
To: linux-kernel, linux-tegra
Cc: JC Kuo, Kishon Vijay Abraham I, Vinod Koul, Thierry Reding,
Jonathan Hunter
If, for some reason, the xusb PHY fails to probe, it leaves
a dangling pointer attached to the platform device structure.
This would normally be harmless, but the Tegra XHCI driver then
goes and extract that pointer from the PHY device. Things go
downhill from there:
8.752082] [004d554e5145533c] address between user and kernel address ranges
[ 8.752085] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 8.752088] Modules linked in: max77620_regulator(E+) xhci_tegra(E+) sdhci_tegra(E+) xhci_hcd(E) sdhci_pltfm(E) cqhci(E) fixed(E) usbcore(E) scsi_mod(E) sdhci(E) host1x(E+)
[ 8.752103] CPU: 4 PID: 158 Comm: systemd-udevd Tainted: G S W E 5.9.0-rc7-00298-gf6337624c4fe #1980
[ 8.752105] Hardware name: NVIDIA Jetson TX2 Developer Kit (DT)
[ 8.752108] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--)
[ 8.752115] pc : kobject_put+0x1c/0x21c
[ 8.752120] lr : put_device+0x20/0x30
[ 8.752121] sp : ffffffc012eb3840
[ 8.752122] x29: ffffffc012eb3840 x28: ffffffc010e82638
[ 8.752125] x27: ffffffc008d56440 x26: 0000000000000000
[ 8.752128] x25: ffffff81eb508200 x24: 0000000000000000
[ 8.752130] x23: ffffff81eb538800 x22: 0000000000000000
[ 8.752132] x21: 00000000fffffdfb x20: ffffff81eb538810
[ 8.752134] x19: 3d4d554e51455300 x18: 0000000000000020
[ 8.752136] x17: ffffffc008d00270 x16: ffffffc008d00c94
[ 8.752138] x15: 0000000000000004 x14: ffffff81ebd4ae90
[ 8.752140] x13: 0000000000000000 x12: ffffff81eb86a4e8
[ 8.752142] x11: ffffff81eb86a480 x10: ffffff81eb862fea
[ 8.752144] x9 : ffffffc01055fb28 x8 : ffffff81eb86a4a8
[ 8.752146] x7 : 0000000000000001 x6 : 0000000000000001
[ 8.752148] x5 : ffffff81dff8bc38 x4 : 0000000000000000
[ 8.752150] x3 : 0000000000000001 x2 : 0000000000000001
[ 8.752152] x1 : 0000000000000002 x0 : 3d4d554e51455300
[ 8.752155] Call trace:
[ 8.752157] kobject_put+0x1c/0x21c
[ 8.752160] put_device+0x20/0x30
[ 8.752164] tegra_xusb_padctl_put+0x24/0x3c
[ 8.752170] tegra_xusb_probe+0x8b0/0xd10 [xhci_tegra]
[ 8.752174] platform_drv_probe+0x60/0xb4
[ 8.752176] really_probe+0xf0/0x504
[ 8.752179] driver_probe_device+0x100/0x170
[ 8.752181] device_driver_attach+0xcc/0xd4
[ 8.752183] __driver_attach+0xb0/0x17c
[ 8.752185] bus_for_each_dev+0x7c/0xd4
[ 8.752187] driver_attach+0x30/0x3c
[ 8.752189] bus_add_driver+0x154/0x250
[ 8.752191] driver_register+0x84/0x140
[ 8.752193] __platform_driver_register+0x54/0x60
[ 8.752197] tegra_xusb_init+0x40/0x1000 [xhci_tegra]
[ 8.752201] do_one_initcall+0x54/0x2d0
[ 8.752205] do_init_module+0x68/0x29c
[ 8.752207] load_module+0x2178/0x26c0
[ 8.752209] __do_sys_finit_module+0xb0/0x120
[ 8.752211] __arm64_sys_finit_module+0x2c/0x40
[ 8.752215] el0_svc_common.constprop.0+0x80/0x240
[ 8.752218] do_el0_svc+0x30/0xa0
[ 8.752220] el0_svc+0x18/0x50
[ 8.752223] el0_sync_handler+0x90/0x318
[ 8.752225] el0_sync+0x158/0x180
[ 8.752230] Code: a9bd7bfd 910003fd a90153f3 aa0003f3 (3940f000)
[ 8.752232] ---[ end trace 90f6c89d62d85ff5 ]---
Reset the pointer on probe failure fixes the issue.
Fixes: 53d2a715c2403 ("phy: Add Tegra XUSB pad controller support")
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
drivers/phy/tegra/xusb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
index de4a46fe1763..ad88d74c1884 100644
--- a/drivers/phy/tegra/xusb.c
+++ b/drivers/phy/tegra/xusb.c
@@ -1242,6 +1242,7 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
reset:
reset_control_assert(padctl->rst);
remove:
+ platform_set_drvdata(pdev, NULL);
soc->ops->remove(padctl);
return err;
}
--
2.28.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] phy: tegra: xusb: Fix dangling pointer on probe failure
2020-10-13 9:58 [PATCH] phy: tegra: xusb: Fix dangling pointer on probe failure Marc Zyngier
@ 2020-10-28 16:12 ` Vinod Koul
2020-10-28 16:45 ` Thierry Reding
1 sibling, 0 replies; 3+ messages in thread
From: Vinod Koul @ 2020-10-28 16:12 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, linux-tegra, JC Kuo, Kishon Vijay Abraham I,
Thierry Reding, Jonathan Hunter
On 13-10-20, 10:58, Marc Zyngier wrote:
> If, for some reason, the xusb PHY fails to probe, it leaves
> a dangling pointer attached to the platform device structure.
>
> This would normally be harmless, but the Tegra XHCI driver then
> goes and extract that pointer from the PHY device. Things go
> downhill from there:
Applied, thanks
--
~Vinod
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] phy: tegra: xusb: Fix dangling pointer on probe failure
2020-10-13 9:58 [PATCH] phy: tegra: xusb: Fix dangling pointer on probe failure Marc Zyngier
2020-10-28 16:12 ` Vinod Koul
@ 2020-10-28 16:45 ` Thierry Reding
1 sibling, 0 replies; 3+ messages in thread
From: Thierry Reding @ 2020-10-28 16:45 UTC (permalink / raw)
To: Marc Zyngier
Cc: linux-kernel, linux-tegra, JC Kuo, Kishon Vijay Abraham I,
Vinod Koul, Jonathan Hunter
[-- Attachment #1: Type: text/plain, Size: 4585 bytes --]
On Tue, Oct 13, 2020 at 10:58:20AM +0100, Marc Zyngier wrote:
> If, for some reason, the xusb PHY fails to probe, it leaves
> a dangling pointer attached to the platform device structure.
>
> This would normally be harmless, but the Tegra XHCI driver then
> goes and extract that pointer from the PHY device. Things go
> downhill from there:
>
> 8.752082] [004d554e5145533c] address between user and kernel address ranges
> [ 8.752085] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [ 8.752088] Modules linked in: max77620_regulator(E+) xhci_tegra(E+) sdhci_tegra(E+) xhci_hcd(E) sdhci_pltfm(E) cqhci(E) fixed(E) usbcore(E) scsi_mod(E) sdhci(E) host1x(E+)
> [ 8.752103] CPU: 4 PID: 158 Comm: systemd-udevd Tainted: G S W E 5.9.0-rc7-00298-gf6337624c4fe #1980
> [ 8.752105] Hardware name: NVIDIA Jetson TX2 Developer Kit (DT)
> [ 8.752108] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--)
> [ 8.752115] pc : kobject_put+0x1c/0x21c
> [ 8.752120] lr : put_device+0x20/0x30
> [ 8.752121] sp : ffffffc012eb3840
> [ 8.752122] x29: ffffffc012eb3840 x28: ffffffc010e82638
> [ 8.752125] x27: ffffffc008d56440 x26: 0000000000000000
> [ 8.752128] x25: ffffff81eb508200 x24: 0000000000000000
> [ 8.752130] x23: ffffff81eb538800 x22: 0000000000000000
> [ 8.752132] x21: 00000000fffffdfb x20: ffffff81eb538810
> [ 8.752134] x19: 3d4d554e51455300 x18: 0000000000000020
> [ 8.752136] x17: ffffffc008d00270 x16: ffffffc008d00c94
> [ 8.752138] x15: 0000000000000004 x14: ffffff81ebd4ae90
> [ 8.752140] x13: 0000000000000000 x12: ffffff81eb86a4e8
> [ 8.752142] x11: ffffff81eb86a480 x10: ffffff81eb862fea
> [ 8.752144] x9 : ffffffc01055fb28 x8 : ffffff81eb86a4a8
> [ 8.752146] x7 : 0000000000000001 x6 : 0000000000000001
> [ 8.752148] x5 : ffffff81dff8bc38 x4 : 0000000000000000
> [ 8.752150] x3 : 0000000000000001 x2 : 0000000000000001
> [ 8.752152] x1 : 0000000000000002 x0 : 3d4d554e51455300
> [ 8.752155] Call trace:
> [ 8.752157] kobject_put+0x1c/0x21c
> [ 8.752160] put_device+0x20/0x30
> [ 8.752164] tegra_xusb_padctl_put+0x24/0x3c
> [ 8.752170] tegra_xusb_probe+0x8b0/0xd10 [xhci_tegra]
> [ 8.752174] platform_drv_probe+0x60/0xb4
> [ 8.752176] really_probe+0xf0/0x504
> [ 8.752179] driver_probe_device+0x100/0x170
> [ 8.752181] device_driver_attach+0xcc/0xd4
> [ 8.752183] __driver_attach+0xb0/0x17c
> [ 8.752185] bus_for_each_dev+0x7c/0xd4
> [ 8.752187] driver_attach+0x30/0x3c
> [ 8.752189] bus_add_driver+0x154/0x250
> [ 8.752191] driver_register+0x84/0x140
> [ 8.752193] __platform_driver_register+0x54/0x60
> [ 8.752197] tegra_xusb_init+0x40/0x1000 [xhci_tegra]
> [ 8.752201] do_one_initcall+0x54/0x2d0
> [ 8.752205] do_init_module+0x68/0x29c
> [ 8.752207] load_module+0x2178/0x26c0
> [ 8.752209] __do_sys_finit_module+0xb0/0x120
> [ 8.752211] __arm64_sys_finit_module+0x2c/0x40
> [ 8.752215] el0_svc_common.constprop.0+0x80/0x240
> [ 8.752218] do_el0_svc+0x30/0xa0
> [ 8.752220] el0_svc+0x18/0x50
> [ 8.752223] el0_sync_handler+0x90/0x318
> [ 8.752225] el0_sync+0x158/0x180
> [ 8.752230] Code: a9bd7bfd 910003fd a90153f3 aa0003f3 (3940f000)
> [ 8.752232] ---[ end trace 90f6c89d62d85ff5 ]---
>
> Reset the pointer on probe failure fixes the issue.
>
> Fixes: 53d2a715c2403 ("phy: Add Tegra XUSB pad controller support")
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
> drivers/phy/tegra/xusb.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
> index de4a46fe1763..ad88d74c1884 100644
> --- a/drivers/phy/tegra/xusb.c
> +++ b/drivers/phy/tegra/xusb.c
> @@ -1242,6 +1242,7 @@ static int tegra_xusb_padctl_probe(struct platform_device *pdev)
> reset:
> reset_control_assert(padctl->rst);
> remove:
> + platform_set_drvdata(pdev, NULL);
> soc->ops->remove(padctl);
> return err;
> }
Sorry, I had missed this before. Why is this necessary? The driver core
already does dev_set_drvdata(dev, NULL) on failure, which is the same as
your platform_set_drvdata() here.
I suppose one possible explanation would be if for some reason we end up
here in the error cleanup path but with err == 0.
Do you have more information on when this happens so that I can repro
and investigate? Alternatively, if you've still got this set up, can you
do a quick test to see if "err" is indeed a negative error code when we
get here?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-29 1:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13 9:58 [PATCH] phy: tegra: xusb: Fix dangling pointer on probe failure Marc Zyngier
2020-10-28 16:12 ` Vinod Koul
2020-10-28 16:45 ` Thierry Reding
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).