Re: [PATCH v2] stm class: Fix a double free in stm_register_device()

[Amelie Delaunay <amelie.delaunay@xxxxxxxxxxx>]

Hi Dan,

On 10/25/23 09:20, Dan Carpenter wrote:
> The put_device(&stm->dev) call will trigger stm_device_release() which
> frees "stm" so the vfree(stm) on the next line is a double free.
>
> Fixes: 389b6699a2aa ("stm class: Fix stm device initialization order")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

Reviewed-by: Amelie Delaunay <amelie.delaunay@xxxxxxxxxxx>

> ---
> v2:  My first version introduced a use after free bug.  Thanks Amelie,
>       for catching this.
>
>   drivers/hwtracing/stm/core.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
> index 534fbefc7f6a..20895d391562 100644
> --- a/drivers/hwtracing/stm/core.c
> +++ b/drivers/hwtracing/stm/core.c
> @@ -868,8 +868,11 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
>   		return -ENOMEM;
>   
>   	stm->major = register_chrdev(0, stm_data->name, &stm_fops);
> -	if (stm->major < 0)
> -		goto err_free;
> +	if (stm->major < 0) {
> +		err = stm->major;
> +		vfree(stm);
> +		return err;
> +	}
>   
>   	device_initialize(&stm->dev);
>   	stm->dev.devt = MKDEV(stm->major, 0);
> @@ -913,10 +916,8 @@ int stm_register_device(struct device *parent, struct stm_data *stm_data,
>   err_device:
>   	unregister_chrdev(stm->major, stm_data->name);
>   
> -	/* matches device_initialize() above */
> +	/* calls stm_device_release() */
>   	put_device(&stm->dev);
> -err_free:
> -	vfree(stm);
>   
>   	return err;
>   }

Regards,
Amelie