[lustre-devel] [PATCH] staging: lustre: Fix avoid intensive reconnecting for ko2iblnd patch
gregkh at linuxfoundation.org
Wed Jan 17 06:02:16 PST 2018
On Wed, Jan 17, 2018 at 12:36:19AM +0000, Dilger, Andreas wrote:
> > On Jan 16, 2018, at 09:56, Greg Kroah-Hartman <gregkh at linuxfoundation.org> wrote:
> > On Tue, Jan 16, 2018 at 03:01:49PM +0000, Eremin, Dmitry wrote:
> >> In the original commit 4d99b2581effe115376402e710fbcb1c3c073769
> > Please use the documented way to write this:
> > 4d99b2581eff ("staging: lustre: avoid intensive reconnecting for ko2iblnd")
> >> was missed one hunk. Added it now to avoid issue with use after free.
> > And I do not understand this commit message at all.
> >> Signed-off-by: Dmitry Eremin <Dmitry.Eremin at intel.com>
> >> ---
> >> drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c | 3 ++-
> >> 1 file changed, 2 insertions(+), 1 deletion(-)
> >> diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> index 2ebc484..a15a625 100644
> >> --- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> +++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
> >> @@ -890,7 +890,8 @@ void kiblnd_destroy_conn(struct kib_conn *conn, bool free_conn)
> >> atomic_dec(&net->ibn_nconns);
> >> }
> >> - kfree(conn);
> >> + if (free_conn)
> >> + kfree(conn);
> > This looks really odd, don't you think?
> I'm not sure what the objection is here? There is an argument to this
> this function named "free_conn" which determines if the structure should
> be freed, or if the network connection is just being torn down and
At first glance it really looks like the normal pattern of:
If you don't want to free the variable, set it to NULL.
Even then, this is a horrible function, you should have 2 different
and then just free the variable in the free_conn() function if you were
going to set the free_conn variable, right?
That way no odd code paths are taken, and it's obvious what you are
doing just by reading the code at the callsite.
More information about the lustre-devel