Re: [PATCH v2] ocxl: Fix potential memory leak on context creation

[Greg Kurz]

On Mon, 20 May 2019 09:16:18 +0200
Frederic Barrat <fbar...@linux.ibm.com> wrote:

> If we couldn't fully init a context, we were leaking memory.
> 
> Fixes: b9721d275cc2 ("ocxl: Allow external drivers to use OpenCAPI contexts")

Oops... missed that during review :-\

> Signed-off-by: Frederic Barrat <fbar...@linux.ibm.com>
> ---
> 
> Changelog:
> v2: reset context pointer in case of allocation failure (Andrew)
> 

Alternatively you could change the code to do:

        ctx = kzalloc(sizeof(struct ocxl_context), GFP_KERNEL);
        if (!ctx)
                return -ENOMEM;
        .
        .
        .
        if (pasid < 0) {
                mutex_unlock(&afu->contexts_lock);
                kfree(ctx);
                return pasid;
        }
        .
        .
        .
        *context = ctx;
        return 0;
}

This has the advantage of clearing any risk of side-effect with
*context forever, which is a safer practice IMHO.

Patch is correct anyway, so:

Reviewed-by: Greg Kurz <gr...@kaod.org>

>  drivers/misc/ocxl/context.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
> index bab9c9364184..24e4fb010275 100644
> --- a/drivers/misc/ocxl/context.c
> +++ b/drivers/misc/ocxl/context.c
> @@ -22,6 +22,8 @@ int ocxl_context_alloc(struct ocxl_context **context, 
> struct ocxl_afu *afu,
>                       afu->pasid_base + afu->pasid_max, GFP_KERNEL);
>       if (pasid < 0) {
>               mutex_unlock(&afu->contexts_lock);
> +             kfree(*context);
> +             *context = NULL;
>               return pasid;
>       }
>       afu->pasid_count++;