linux-kernel - Re: [PATCH 2/8] Input: ff-core - make use of _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7d1e1c89-d4a6-4b3d-a674-5ef497c2c496@samsung.com>
Date: Fri, 20 Dec 2024 13:36:59 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>, Jiri Kosina
	<jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>, Hans de Goede
	<hdegoede@...hat.com>
Cc: linux-input@...r.kernel.org, linux-kernel@...r.kernel.org, 'Linux
	Samsung SOC' <linux-samsung-soc@...r.kernel.org>
Subject: Re: [PATCH 2/8] Input: ff-core - make use of __free() cleanup
 facility

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@...il.com>
> ---
>   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