Re: [PATCH v2] misc: sram: Fix NULL pointer dereference in sram_probe

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Mar 07, 2025 at 05:34:42PM +0300, Andrey Tsygunka wrote:
> Add a check for the return value from platform_get_resource() call
> to be NULL.
> 
> If the passed device-tree contains a node for sram-device
> without a specified '<reg>' property value, for example:
> 
>     sram: sram@5c0000000 {
>         compatible = "nvidia,tegra186-sysram";
>     };
> 
> and the of_device_id[] '.data' element contains a sram_config*
> with '.map_only_reserved = true' property, we get the error:
> 
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.96 #1
> Hardware name: linux,dummy-virt (DT)
> Call trace:
>   sram_probe+0x134/0xd30
>   platform_probe+0x94/0x130
>   really_probe+0x124/0x580
>   __driver_probe_device+0xd0/0x1f0
>   driver_probe_device+0x50/0x1c0
>   __device_attach_driver+0x140/0x220
>   bus_for_each_drv+0xbc/0x130
>   __device_attach+0xec/0x2c0
>   device_initial_probe+0x24/0x40
>   bus_probe_device+0xd8/0xe0
>   device_add+0x67c/0xc80
>   of_device_add+0x58/0x80
>   of_platform_device_create_pdata+0xd0/0x1b0
>   of_platform_bus_create+0x27c/0x6f0
>   of_platform_populate+0xac/0x1d0
>   of_platform_default_populate_init+0x10c/0x130
>   do_one_initcall+0xdc/0x510
>   kernel_init_freeable+0x43c/0x4d8
>   kernel_init+0x2c/0x1e0
>   ret_from_fork+0x10/0x20
> 
> Fixes: 444b0111f3bc ("misc: sram: use devm_platform_ioremap_resource_wc()")
> Signed-off-by: Andrey Tsygunka <aitsygunka@yandex.ru>
> ---
> v2: Description changed based on comments from Markus Elfring 
> at https://lore.kernel.org/linux-kernel/84969aba-67ba-4990-9065-6b55ce26ff92@web.de/,
> added tag 'Fixes', removed useless information from backtrace.
> 
>  drivers/misc/sram.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/sram.c b/drivers/misc/sram.c
> index e5069882457e..c8ba8ebd4364 100644
> --- a/drivers/misc/sram.c
> +++ b/drivers/misc/sram.c
> @@ -410,8 +410,13 @@ static int sram_probe(struct platform_device *pdev)
>  	if (IS_ERR(clk))
>  		return PTR_ERR(clk);
>  
> -	ret = sram_reserve_regions(sram,
> -			platform_get_resource(pdev, IORESOURCE_MEM, 0));
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (unlikely(res == NULL)) {

Only use likely/unlikely if you can actually benchmark the difference.
For probe() callbacks, that is never the case, so just rely on the
compiler and cpu to get it right (hint, it almost always will.)

thanks,

greg k-h