linux-kernel - Re: [PATCH 1/3] Input: goodix - Fix double free on managed resource

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Sat, 23 Apr 2022 13:34:46 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     José Expósito <jose.exposito89@...il.com>,
        hadess@...ess.net
Cc:     dmitry.torokhov@...il.com, rydberg@...math.org, lains@...eup.net,
        jikos@...nel.org, benjamin.tissoires@...hat.com,
        linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] Input: goodix - Fix double free on managed resource

Hi José,

On 4/22/22 18:17, José Expósito wrote:
> As described in the documentation for devm_input_allocate_device():
> 
>   Managed input devices do not need to be explicitly unregistered or
>   freed as it will be done automatically when owner device unbinds from
>   its driver (or binding fails).
> 
> However this driver was explicitly freeing the input device.
> 
> Remove the calls to input_free_device() to avoid a possible double free
> error.
> 
> Fixes: 5ede7f0cfb93f ("Input: goodix - add pen support")
> Signed-off-by: José Expósito <jose.exposito89@...il.com>
> ---
>  drivers/input/touchscreen/goodix.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
> index 752e8ba4fecb..61eb69f3a259 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -308,10 +308,8 @@ static struct input_dev *goodix_create_pen_input(struct goodix_ts_data *ts)
>  		return NULL;
>  
>  	input_alloc_absinfo(input);
> -	if (!input->absinfo) {
> -		input_free_device(input);
> +	if (!input->absinfo)
>  		return NULL;
> -	}
>  
>  	input->absinfo[ABS_X] = ts->input_dev->absinfo[ABS_MT_POSITION_X];
>  	input->absinfo[ABS_Y] = ts->input_dev->absinfo[ABS_MT_POSITION_Y];

I don't know what tree you've based this on, but the above code has been replaced
with the new input_copy_abs helper in Linus' current master branch:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/touchscreen/goodix.c#n310

> @@ -340,10 +338,8 @@ static struct input_dev *goodix_create_pen_input(struct goodix_ts_data *ts)
>  		input->id.product = 0x1001;
>  	input->id.version = ts->version;
>  
> -	if (input_register_device(input) != 0) {
> -		input_free_device(input);
> +	if (input_register_device(input) != 0)
>  		return NULL;
> -	}
>  
>  	return input;
>  }

And this code has also already been fixed, so this patch can be dropped.

Regards,

Hans