On 07.11.2024 08:15, Dmitry Torokhov wrote:
> Annotate allocated memory with __free(kfree) to simplify the code and
> make sure memory is released appropriately.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> ---
> drivers/input/ff-core.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
This patch landed in linux-next as commit 5203b3a18c1b ("Input: ff-core
- make use of __free() cleanup facility"). In my tests I found that it
causes the following kernel panic on some of my test boards. Reverting
it, together with fd5ba0501d38 ("Input: ff-memless - make use of
__free() cleanup facility") on top of next-20241220 fixes this panic
issue. Here is the relevant log captured on Samsung Exynos4412 ARM
32bit-based Trats2 board:
8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address
00000024 when read
[00000024] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted
6.13.0-rc3-next-20241220 #15500
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at input_ff_create+0xa0/0x13c
LR is at input_ff_create+0xb8/0x13c
pc : [<c08d7e14>] lr : [<c08d7e2c>] psr: 80000013
...
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
...
Call trace:
input_ff_create from input_ff_create_memless+0x8c/0x160
input_ff_create_memless from max77693_haptic_probe+0x1b0/0x284
max77693_haptic_probe from platform_probe+0x80/0xc0
platform_probe from really_probe+0x154/0x3ac
really_probe from __driver_probe_device+0xa0/0x1d4
__driver_probe_device from driver_probe_device+0x30/0xd0
driver_probe_device from __driver_attach+0x10c/0x190
__driver_attach from bus_for_each_dev+0x60/0xb4
bus_for_each_dev from bus_add_driver+0xe0/0x220
bus_add_driver from driver_register+0x7c/0x118
driver_register from do_one_initcall+0x6c/0x328
do_one_initcall from kernel_init_freeable+0x1c8/0x218
kernel_init_freeable from kernel_init+0x1c/0x12c
kernel_init from ret_from_fork+0x14/0x28
Exception stack(0xf0845fb0 to 0xf0845ff8)
...
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
---[ end Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b ]---
> diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c
> index eb01bcb69d00..a235d2eb6b31 100644
> --- a/drivers/input/ff-core.c
> +++ b/drivers/input/ff-core.c
> @@ -290,8 +290,6 @@ EXPORT_SYMBOL_GPL(input_ff_event);
> */
> int input_ff_create(struct input_dev *dev, unsigned int max_effects)
> {
> - struct ff_device *ff;
> - size_t ff_dev_size;
> int i;
>
> if (!max_effects) {
> @@ -304,25 +302,20 @@ int input_ff_create(struct input_dev *dev, unsigned int max_effects)
> return -EINVAL;
> }
>
> - ff_dev_size = struct_size(ff, effect_owners, max_effects);
> - if (ff_dev_size == SIZE_MAX) /* overflow */
> - return -EINVAL;
> -
> - ff = kzalloc(ff_dev_size, GFP_KERNEL);
> + struct ff_device *ff __free(kfree) =
> + kzalloc(struct_size(ff, effect_owners, max_effects),
> + GFP_KERNEL);
> if (!ff)
> return -ENOMEM;
>
> - ff->effects = kcalloc(max_effects, sizeof(struct ff_effect),
> - GFP_KERNEL);
> - if (!ff->effects) {
> - kfree(ff);
> + ff->effects = kcalloc(max_effects, sizeof(*ff->effects), GFP_KERNEL);
> + if (!ff->effects)
> return -ENOMEM;
> - }
>
> ff->max_effects = max_effects;
> mutex_init(&ff->mutex);
>
> - dev->ff = ff;
> + dev->ff = no_free_ptr(ff);
> dev->flush = input_ff_flush;
> dev->event = input_ff_event;
> __set_bit(EV_FF, dev->evbit);
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
