Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c4f8bfef-1cb6-d7bb-dfe1-04b3d952738a@redhat.com>
Date:   Sat, 23 Apr 2022 13:34:46 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH 1/3] Input: goodix - Fix double free on managed resource
Content-Language: en-US
To:     =?UTF-8?B?Sm9zw6kgRXhww7NzaXRv?= <jose.exposito89@gmail.com>,
        hadess@hadess.net
Cc:     dmitry.torokhov@gmail.com, rydberg@bitmath.org, lains@riseup.net,
        jikos@kernel.org, benjamin.tissoires@redhat.com,
        linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220422161709.30198-1-jose.exposito89@gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20220422161709.30198-1-jose.exposito89@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@gmail.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