Re: [PATCH] soc/tegra: fuse: Fix crash in tegra_fuse_readl()

[Jon Hunter <jonathanh@nvidia.com>]

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

-- 
nvpublic