[PATCH] 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] misc: sram: Fix NULL pointer dereference in sram_probe
Posted by Andrey Tsygunka 11 months, 1 week ago
Added check for res for NULL value.
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:

[    2.130808][    T1] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[    2.133389][    T1] Mem abort info:
[    2.134319][    T1]   ESR = 0x0000000096000004
[    2.135484][    T1]   EC = 0x25: DABT (current EL), IL = 32 bits
[    2.136816][    T1]   SET = 0, FnV = 0
[    2.137883][    T1]   EA = 0, S1PTW = 0
[    2.138954][    T1]   FSC = 0x04: level 0 translation fault
[    2.140203][    T1] Data abort info:
[    2.141162][    T1]   ISV = 0, ISS = 0x00000004
[    2.142246][    T1]   CM = 0, WnR = 0
[    2.144038][    T1] [0000000000000000] user address but active_mm is swapper
[    2.146003][    T1] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[    2.147589][    T1] Modules linked in:
[    2.148735][    T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.1.96 #1
[    2.150051][    T1] Hardware name: linux,dummy-virt (DT)
[    2.151492][    T1] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    2.152996][    T1] pc : sram_probe+0x134/0xd30
[    2.154517][    T1] lr : sram_probe+0x114/0xd30
[    2.155710][    T1] sp : ffff80000efdb820
[    2.156443][    T1] x29: ffff80000efdb8d0 x28: 0000000000000000 x27: ffff80000efdb878
[    2.158173][    T1] x26: 0000000000000000 x25: ffff0000ff816bc8 x24: 0000000000000000
[    2.159828][    T1] x23: ffff0000c0cb0480 x22: ffff8000099be080 x21: ffff0000c0bc4000
[    2.161554][    T1] x20: ffff80000c14cac8 x19: fffffffffffffffe x18: 0000000000000000
[    2.163148][    T1] x17: 203d20647561625f x16: 65736162202c3331 x15: 0000000000000028
[    2.164850][    T1] x14: 0000000000000d2e x13: 0000000000000d2f x12: ffff80000e410d00
[    2.166514][    T1] x11: 0000000000000003 x10: ffff80000ec93074 x9 : ffff80000e406000
[    2.168194][    T1] x8 : ffff80000efdb518 x7 : ffff0000c0a50000 x6 : 0000000000000000
[    2.169306][    T1] x5 : ffff0000c0a50000 x4 : 0000000000000000 x3 : ffff800009946e88
[    2.170646][    T1] x2 : ffff0000ff816bb0 x1 : ffff0000c0bc4010 x0 : 0000000000000000
[    2.172457][    T1] Call trace:
[    2.173114][    T1]  sram_probe+0x134/0xd30
[    2.174334][    T1]  platform_probe+0x94/0x130
[    2.175589][    T1]  really_probe+0x124/0x580
[    2.176706][    T1]  __driver_probe_device+0xd0/0x1f0
[    2.177885][    T1]  driver_probe_device+0x50/0x1c0
[    2.179037][    T1]  __device_attach_driver+0x140/0x220
[    2.180274][    T1]  bus_for_each_drv+0xbc/0x130
[    2.181423][    T1]  __device_attach+0xec/0x2c0
[    2.182580][    T1]  device_initial_probe+0x24/0x40
[    2.183734][    T1]  bus_probe_device+0xd8/0xe0
[    2.184826][    T1]  device_add+0x67c/0xc80
[    2.185800][    T1]  of_device_add+0x58/0x80
[    2.186752][    T1]  of_platform_device_create_pdata+0xd0/0x1b0
[    2.187923][    T1]  of_platform_bus_create+0x27c/0x6f0
[    2.188998][    T1]  of_platform_populate+0xac/0x1d0
[    2.190030][    T1]  of_platform_default_populate_init+0x10c/0x130
[    2.191409][    T1]  do_one_initcall+0xdc/0x510
[    2.192441][    T1]  kernel_init_freeable+0x43c/0x4d8
[    2.193485][    T1]  kernel_init+0x2c/0x1e0
[    2.194496][    T1]  ret_from_fork+0x10/0x20
[    2.195972][    T1] Code: f9002bff f90033fb f941e822 f90003e2 (a9400001)
[    2.197354][    T1] ---[ end trace 0000000000000000 ]---
[    2.198333][    T1] Kernel panic - not syncing: Oops: Fatal exception

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Andrey Tsygunka <aitsygunka@yandex.ru>
---
 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] misc: sram: Fix NULL pointer dereference in sram_probe()
Posted by Markus Elfring 11 months, 1 week ago
> Added check for res for NULL value.
…

Please improve such a change description another bit.

See also:
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc5#n94

Regards,
Markus
[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