[PATCH] drivers: input: input: Fix possible sleep-in-atomic bug in input_alloc_absinfo()

starmiku1207184332@gmail.com posted 1 patch 2 years, 9 months ago
drivers/input/input.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] drivers: input: input: Fix possible sleep-in-atomic bug in input_alloc_absinfo()
Posted by starmiku1207184332@gmail.com 2 years, 9 months ago
From: Teng Qi <starmiku1207184332@gmail.com>

input_alloc_absinfo() may cause the kernel to sleep by calling kcalloc() with
the GFP_KERNEL flag. It may be called by input_event() holding spinlock through
the following call path: 

input_event(), 433
input_handle_event(), 399
input_get_disposition(), 294
input_handle_abs_event(), 232
input_abs_set_val(), Clang suggests it calls input_alloc_absinfo()
	unconditionally
input_alloc_absinfo(), 483
kcalloc(..., GFP_KERNEL)

This call path can potentially trigger a sleep-in-atomic bug. To prevent this
potential bug, we modify input_alloc_absinfo() to use the GFP_ATOMIC flag
instead of the GFP_KERNEL flag.
The possible bug is detected by a static code analysis tool.

Signed-off-by: Teng Qi <starmiku1207184332@gmail.com>
---
 drivers/input/input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 37e876d45eb9..eefc4c85a693 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -480,7 +480,7 @@ void input_alloc_absinfo(struct input_dev *dev)
 	if (dev->absinfo)
 		return;
 
-	dev->absinfo = kcalloc(ABS_CNT, sizeof(*dev->absinfo), GFP_KERNEL);
+	dev->absinfo = kcalloc(ABS_CNT, sizeof(*dev->absinfo), GFP_ATOMIC);
 	if (!dev->absinfo) {
 		dev_err(dev->dev.parent ?: &dev->dev,
 			"%s: unable to allocate memory\n", __func__);
-- 
2.25.1
Re: [PATCH] drivers: input: input: Fix possible sleep-in-atomic bug in input_alloc_absinfo()
Posted by Dmitry Torokhov 2 years, 9 months ago
Hi Teng Qi,

On Tue, Apr 25, 2023 at 07:49:51AM +0000, starmiku1207184332@gmail.com wrote:
> From: Teng Qi <starmiku1207184332@gmail.com>
> 
> input_alloc_absinfo() may cause the kernel to sleep by calling kcalloc() with
> the GFP_KERNEL flag. It may be called by input_event() holding spinlock through
> the following call path: 
> 
> input_event(), 433
> input_handle_event(), 399
> input_get_disposition(), 294
> input_handle_abs_event(), 232
> input_abs_set_val(), Clang suggests it calls input_alloc_absinfo()
> 	unconditionally
> input_alloc_absinfo(), 483
> kcalloc(..., GFP_KERNEL)
> 
> This call path can potentially trigger a sleep-in-atomic bug. To prevent this
> potential bug, we modify input_alloc_absinfo() to use the GFP_ATOMIC flag
> instead of the GFP_KERNEL flag.

This can not happen in practice, and changing the allocation type to
GFP_ATOMIC hurts the other code. We can avoid the issue by not using the
helper and have input core access the slot info directly. I CCed you on
a patch doing so.

> The possible bug is detected by a static code analysis tool.
> 
> Signed-off-by: Teng Qi <starmiku1207184332@gmail.com>
> ---
>  drivers/input/input.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 37e876d45eb9..eefc4c85a693 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -480,7 +480,7 @@ void input_alloc_absinfo(struct input_dev *dev)
>  	if (dev->absinfo)
>  		return;
>  
> -	dev->absinfo = kcalloc(ABS_CNT, sizeof(*dev->absinfo), GFP_KERNEL);
> +	dev->absinfo = kcalloc(ABS_CNT, sizeof(*dev->absinfo), GFP_ATOMIC);
>  	if (!dev->absinfo) {
>  		dev_err(dev->dev.parent ?: &dev->dev,
>  			"%s: unable to allocate memory\n", __func__);
> -- 
> 2.25.1
> 

Thanks.

-- 
Dmitry