Re: [PATCH] misc: ocxl: fix possible name leak in ocxl_file_register_afu()

[Frederic Barrat <fbarrat@linux.ibm.com>]

On 11/11/2022 15:59, Yang Yingliang wrote:
> If device_register() returns error in ocxl_file_register_afu(),
> the name allocated by dev_set_name() need be freed. As comment
> of device_register() says, it should use put_device() to give
> up the reference in the error path. So fix this by calling
> put_device(), then the name can be freed in kobject_cleanup(),
> and info is freed in info_release().
> 
> Fixes: 75ca758adbaf ("ocxl: Create a clear delineation between ocxl backend & frontend")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>   drivers/misc/ocxl/file.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index d46dba2df5a1..452d5777a0e4 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -541,8 +541,11 @@ int ocxl_file_register_afu(struct ocxl_afu *afu)
>   		goto err_put;
>   
>   	rc = device_register(&info->dev);
> -	if (rc)
> -		goto err_put;
> +	if (rc) {
> +		free_minor(info);
> +		put_device(&info->dev);
> +		return rc;
> +	}


While I agree that a put_device() is needed on that error path, the fix 
above is not correct as it forgets to release the afu reference and the 
memory allocated in info. That was taken care of by the jump to the 
err_put label, so it should be kept. Something like:

-	if (rc)
+	if (rc) {
+		put_device((&info->dev);
  		goto err_put;
+	}


   Fred