| Message ID | 20240129120151.16198-1-jonathanh@nvidia.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | soc/tegra: fuse: Fix crash in tegra_fuse_readl() | expand |
On 29/01/2024 12:01, Jon Hunter wrote: > Commit c5b2d43e67bb ("soc/tegra: fuse: Add ACPI support for Tegra194 and > Tegra234") updated the Tegra fuse driver to add ACPI support and added a > test to the tegra_fuse_readl() function to check if the device is > booting with device-tree. This test passes 'fuse->dev' variable to > dev_fwnode() but does not check first is 'fuse->dev' is valid. This is > causing a crash to occur in Tegra XUSB PHY driver that calls the > tegra_fuse_readl() function before 'fuse->dev' variable has been > initialised ... > > Unable to handle kernel NULL pointer dereference at virtual address 0000000000000290 > Mem abort info: > ESR = 0x0000000096000004 > EC = 0x25: DABT (current EL), IL = 32 bits > SET = 0, FnV = 0 > EA = 0, S1PTW = 0 > FSC = 0x04: level 0 translation fault > Data abort info: > ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > [0000000000000290] user address but active_mm is swapper > Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP > Modules linked in: > CPU: 7 PID: 70 Comm: kworker/u16:4 Not tainted 6.8.0-rc1-next-20240129-02825-g596764183be8 #1 > Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT) > Workqueue: events_unbound deferred_probe_work_func > pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > pc : __dev_fwnode+0x0/0x18 > lr : tegra_fuse_readl+0x24/0x98 > sp : ffff80008393ba10 > x29: ffff80008393ba10 x28: 0000000000000000 x27: ffff800081233c10 > x26: 00000000000001c8 x25: ffff000080b7bc10 x24: ffff000082df3b00 > x23: fffffffffffffff4 x22: 0000000000000004 x21: ffff80008393ba84 > x20: 00000000000000f0 x19: ffff800082f1e000 x18: ffff800081d72000 > x17: 0000000000000001 x16: 0000000000000001 x15: ffff800082fcdfff > x14: 0000000000000000 x13: 0000000003541000 x12: 0000000000000020 > x11: 0140000000000000 x10: ffff800080000000 x9 : 0000000000000000 > x8 : ffff000082df3b40 x7 : 0000000000000000 x6 : 000000000000003f > x5 : 00000000ffffffff x4 : 0000000000000dc0 x3 : 00000000000000c0 > x2 : 0000000000000001 x1 : ffff80008393ba84 x0 : 0000000000000000 > Call trace: > __dev_fwnode+0x0/0x18 > tegra186_xusb_padctl_probe+0xb0/0x1a8 > tegra_xusb_padctl_probe+0x7c/0xebc > platform_probe+0x90/0xd8 > really_probe+0x13c/0x29c > __driver_probe_device+0x7c/0x124 > driver_probe_device+0x38/0x11c > __device_attach_driver+0x90/0xdc > bus_for_each_drv+0x78/0xdc > __device_attach+0xfc/0x188 > device_initial_probe+0x10/0x18 > bus_probe_device+0xa4/0xa8 > deferred_probe_work_func+0x80/0xb4 > process_scheduled_works+0x178/0x3e0 > worker_thread+0x164/0x2e8 > kthread+0xfc/0x11c > ret_from_fork+0x10/0x20 > Code: a8c27bfd d65f03c0 128002a0 d65f03c0 (f9414801) > ---[ end trace 0000000000000000 ]--- > > Fix this by verifying that 'fuse->dev' is valid before passing to > dev_fwnode(). Given that 'fuse->clk' and 'fuse->clk' are initialised in > the same function, test these variables in the same if-statement. > > Fixes: c5b2d43e67bb ("soc/tegra: fuse: Add ACPI support for Tegra194 and Tegra234") > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > drivers/soc/tegra/fuse/fuse-tegra.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c > index c34efa5bf44c..efe9853d4a8a 100644 > --- a/drivers/soc/tegra/fuse/fuse-tegra.c > +++ b/drivers/soc/tegra/fuse/fuse-tegra.c > @@ -348,7 +348,10 @@ int tegra_fuse_readl(unsigned long offset, u32 *value) > /* > * Wait for fuse->clk to be initialized if device-tree boot is used. > */ > - if (is_of_node(dev_fwnode(fuse->dev)) && !fuse->clk) > + if (!fuse->clk || !fuse->dev) > + return -EPROBE_DEFER; > + > + if (is_of_node(dev_fwnode(fuse->dev))) > return -EPROBE_DEFER; Actually, this is still not correct, because this will now break ACPI support. I will update and resend. Jon
diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c index c34efa5bf44c..efe9853d4a8a 100644 --- a/drivers/soc/tegra/fuse/fuse-tegra.c +++ b/drivers/soc/tegra/fuse/fuse-tegra.c @@ -348,7 +348,10 @@ int tegra_fuse_readl(unsigned long offset, u32 *value) /* * Wait for fuse->clk to be initialized if device-tree boot is used. */ - if (is_of_node(dev_fwnode(fuse->dev)) && !fuse->clk) + if (!fuse->clk || !fuse->dev) + return -EPROBE_DEFER; + + if (is_of_node(dev_fwnode(fuse->dev))) return -EPROBE_DEFER; if (!fuse->read)
Commit c5b2d43e67bb ("soc/tegra: fuse: Add ACPI support for Tegra194 and Tegra234") updated the Tegra fuse driver to add ACPI support and added a test to the tegra_fuse_readl() function to check if the device is booting with device-tree. This test passes 'fuse->dev' variable to dev_fwnode() but does not check first is 'fuse->dev' is valid. This is causing a crash to occur in Tegra XUSB PHY driver that calls the tegra_fuse_readl() function before 'fuse->dev' variable has been initialised ... Unable to handle kernel NULL pointer dereference at virtual address 0000000000000290 Mem abort info: ESR = 0x0000000096000004 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x04: level 0 translation fault Data abort info: ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 CM = 0, WnR = 0, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [0000000000000290] user address but active_mm is swapper Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP Modules linked in: CPU: 7 PID: 70 Comm: kworker/u16:4 Not tainted 6.8.0-rc1-next-20240129-02825-g596764183be8 #1 Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT) Workqueue: events_unbound deferred_probe_work_func pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : __dev_fwnode+0x0/0x18 lr : tegra_fuse_readl+0x24/0x98 sp : ffff80008393ba10 x29: ffff80008393ba10 x28: 0000000000000000 x27: ffff800081233c10 x26: 00000000000001c8 x25: ffff000080b7bc10 x24: ffff000082df3b00 x23: fffffffffffffff4 x22: 0000000000000004 x21: ffff80008393ba84 x20: 00000000000000f0 x19: ffff800082f1e000 x18: ffff800081d72000 x17: 0000000000000001 x16: 0000000000000001 x15: ffff800082fcdfff x14: 0000000000000000 x13: 0000000003541000 x12: 0000000000000020 x11: 0140000000000000 x10: ffff800080000000 x9 : 0000000000000000 x8 : ffff000082df3b40 x7 : 0000000000000000 x6 : 000000000000003f x5 : 00000000ffffffff x4 : 0000000000000dc0 x3 : 00000000000000c0 x2 : 0000000000000001 x1 : ffff80008393ba84 x0 : 0000000000000000 Call trace: __dev_fwnode+0x0/0x18 tegra186_xusb_padctl_probe+0xb0/0x1a8 tegra_xusb_padctl_probe+0x7c/0xebc platform_probe+0x90/0xd8 really_probe+0x13c/0x29c __driver_probe_device+0x7c/0x124 driver_probe_device+0x38/0x11c __device_attach_driver+0x90/0xdc bus_for_each_drv+0x78/0xdc __device_attach+0xfc/0x188 device_initial_probe+0x10/0x18 bus_probe_device+0xa4/0xa8 deferred_probe_work_func+0x80/0xb4 process_scheduled_works+0x178/0x3e0 worker_thread+0x164/0x2e8 kthread+0xfc/0x11c ret_from_fork+0x10/0x20 Code: a8c27bfd d65f03c0 128002a0 d65f03c0 (f9414801) ---[ end trace 0000000000000000 ]--- Fix this by verifying that 'fuse->dev' is valid before passing to dev_fwnode(). Given that 'fuse->clk' and 'fuse->clk' are initialised in the same function, test these variables in the same if-statement. Fixes: c5b2d43e67bb ("soc/tegra: fuse: Add ACPI support for Tegra194 and Tegra234") Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/soc/tegra/fuse/fuse-tegra.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)