[lvc-project] [PATCH] misc: sram: Fix NULL pointer dereference in sram_probe

Fri Mar 7 00:59:02 MSK 2025

Здравствуйте!

[ если ранее не взаимодействовали с патчами в ядро Linux, рекомендуется
  предварительно отправлять их в lvc-patches at linuxtesting.org, где
  некоторые вещи можно обсудить/отладить до отправки исправления в
  международные списки рассылки ]

Несколько моментов по оформлению и сути патчей ниже.

On Wed, 05. Mar 17:50, Andrey Tsygunka wrote:
> Added check for res for NULL value.

  Describe your changes in imperative mood, e.g. “make xyzzy do frotz”
  instead of “[This patch] makes xyzzy do frotz” or “[I] changed xyzzy
  to do frotz”, as if you are giving orders to the codebase to change
  its behaviour.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

Строки обычного текста допускаются до 75 символов. Абзацы разделяют
пустой строкой.

> If the passed device-tree contains a node for sram-device
> without a specified '<reg>' property value, for example:
> 
>     sram: sram at 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

Лучше вставлять только полезную и нужную информацию из логов. Например,
в данном случае информация о временных метках не нужна, содержимое
регистров не важно для понимания проблемы.

Остальное с наименованием ошибки, hardware, kernel version и самим
бэктрейсом ошибки конечно являются важным.
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#backtraces-in-commit-messages

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

Если патч исправляет баг в ядре, то желателен тег Fixes. Если хотите
видеть патч в stable-ветках, тег Cc: stable at vger.kernel.org
https://portal.linuxtesting.ru/How-to-send-patches-to-kernel.html#Fixes
https://portal.linuxtesting.ru/How-to-send-patches-to-kernel.html#Порядок-тегов

> Signed-off-by: Andrey Tsygunka <aitsygunka at 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)) {

Использование likely/unlikely - вмешательство в дела компилятора. Стоит
употреблять только тогда, когда данные подсказки компилятору
действительно оправданны на практике (результаты бенчмарков). Иначе
современные компиляторы разберутся и без того, лишние подсказки наоборот
могут сбить их с толку и даже _ухудшить_ положение дел. Этап пробинга
однозначно нельзя отнести к путям, чувствительным к производительности.
Лучше обойтись без unlikely.

Проверки на NULL обычно принято делать посредством (!pointer), о чём
также сообщает scripts/checkpatch.pl

if (!res) {
	...
}

> +		dev_err(&pdev->dev, "invalid resource\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = sram_reserve_regions(sram, res);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.25.1