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

Subsystems: char and misc drivers, ocxl (open coherent accelerator processor interface opencapi) driver, the rest

STALE2636d

3 messages, 3 authors, 2019-05-20 · open the first message on its own page

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

From: Frederic Barrat <hidden>
Date: 2019-05-20 07:17:43

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 <redacted>
---

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

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

From: Andrew Donnellan <hidden>
Date: 2019-05-20 07:20:05

Acked-by: Andrew Donnellan <redacted>

On 20/5/19 5:16 pm, Frederic Barrat wrote:
quoted hunk
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 <redacted>
---

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

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

From: Greg Kurz <hidden>
Date: 2019-05-20 09:12:50

On Mon, 20 May 2019 09:16:18 +0200
Frederic Barrat [off-list ref] 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 <redacted>
---

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 <redacted>
quoted hunk
 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++;

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help