| Message ID | 20190520071618.1722-1-fbarrat@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] ocxl: Fix potential memory leak on context creation | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (8150a153c013aa2dd1ffae43370b89ac1347a7fb) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
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++; >
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++;
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++;
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(+)