Re: Re: [PATCH] [v2] SUNRPC: fix a memleak in gss_import_v2_context

From: alexious
Date: Wed Jan 17 2024 - 02:54:57 EST



> > On Jan 15, 2024, at 6:09 AM, Simon Horman <horms@xxxxxxxxxx> wrote:
> >
> > On Fri, Jan 12, 2024 at 04:45:38PM +0800, Zhipeng Lu wrote:
> >> The ctx->mech_used.data allocated by kmemdup is not freed in neither
> >> gss_import_v2_context nor it only caller radeon_driver_open_kms.
> >
> > Should radeon_driver_open_kms be gss_krb5_import_sec_context?
> >
> > Also, perhaps it is useful to write something like this:
> >
> > ... gss_krb5_import_sec_context, which frees ctx on error.

Yes, you are right, I proberly mixed up it to another patch :(.
And the first sentence of the patch description should be:

The ctx->mech_used.data allocated by kmemdup is not freed in neither
gss_import_v2_context nor it only caller gss_krb5_import_sec_context,
which frees ctx on error.

>
> If Zhipeng agrees to this suggestion, I can change the
> patch description in my tree. A v3 is not necessary.

Yes, I agree with Simon's suggestion and I give the corrected description
above.

>
> >> Thus, this patch reform the last call of gss_import_v2_context to the
> >> gss_krb5_import_ctx_v2, preventing the memleak while keepping the return
> >> formation.
> >>
> >> Fixes: 47d848077629 ("gss_krb5: handle new context format from gssd")
> >> Signed-off-by: Zhipeng Lu <alexious@xxxxxxxxxx>
> >
> > Hi Zhipeng Lu,
> >
> > Other than the comment above, I agree with your analysis.
> > And that although the problem has changed form slightly,
> > it was originally introduced by the cited commit.
> > I also agree that your fix.
> >
> > ...
>
> --
> Chuck Lever
>
>