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

Andrey Tsygunka posted 1 patch 11 months, 1 week ago
There is a newer version of this series
drivers/misc/sram.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[PATCH v2] misc: sram: Fix NULL pointer dereference in sram_probe
Posted by Andrey Tsygunka 11 months, 1 week ago
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)) {
+		dev_err(&pdev->dev, "invalid resource\n");
+		return -EINVAL;
+	}
+
+	ret = sram_reserve_regions(sram, res);
 	if (ret)
 		return ret;
 
-- 
2.25.1
Re: [PATCH v2] misc: sram: Fix NULL pointer dereference in sram_probe
Posted by Greg KH 9 months, 4 weeks ago
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