linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] ocxl: Fix potential memory leak on context creation
@ 2019-05-20  7:16 Frederic Barrat
  2019-05-20  7:18 ` Andrew Donnellan
  2019-05-20  7:55 ` Greg Kurz
  0 siblings, 2 replies; 3+ messages in thread
From: Frederic Barrat @ 2019-05-20  7:16 UTC (permalink / raw)
  To: linuxppc-dev, andrew.donnellan, alastair; +Cc: clombard

If we couldn't fully init a context, we were leaking memory.

Fixes: b9721d275cc2 ("ocxl: Allow external drivers to use OpenCAPI contexts")
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---

Changelog:
v2: reset context pointer in case of allocation failure (Andrew)

 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++;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ocxl: Fix potential memory leak on context creation
  2019-05-20  7:16 [PATCH v2] ocxl: Fix potential memory leak on context creation Frederic Barrat
@ 2019-05-20  7:18 ` Andrew Donnellan
  2019-05-20  7:55 ` Greg Kurz
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Donnellan @ 2019-05-20  7:18 UTC (permalink / raw)
  To: Frederic Barrat, linuxppc-dev, andrew.donnellan, alastair; +Cc: clombard

Acked-by: Andrew Donnellan <ajd@linux.ibm.com>

On 20/5/19 5:16 pm, Frederic Barrat wrote:
> If we couldn't fully init a context, we were leaking memory.
> 
> Fixes: b9721d275cc2 ("ocxl: Allow external drivers to use OpenCAPI contexts")
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
> 
> Changelog:
> v2: reset context pointer in case of allocation failure (Andrew)
> 
>   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++;
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ocxl: Fix potential memory leak on context creation
  2019-05-20  7:16 [PATCH v2] ocxl: Fix potential memory leak on context creation Frederic Barrat
  2019-05-20  7:18 ` Andrew Donnellan
@ 2019-05-20  7:55 ` Greg Kurz
  1 sibling, 0 replies; 3+ messages in thread
From: Greg Kurz @ 2019-05-20  7:55 UTC (permalink / raw)
  To: Frederic Barrat; +Cc: clombard, linuxppc-dev, alastair, andrew.donnellan

On Mon, 20 May 2019 09:16:18 +0200
Frederic Barrat <fbarrat@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 <fbarrat@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 <groug@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++;


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-05-20  9:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20  7:16 [PATCH v2] ocxl: Fix potential memory leak on context creation Frederic Barrat
2019-05-20  7:18 ` Andrew Donnellan
2019-05-20  7:55 ` Greg Kurz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).