lkml.org 
[lkml]   [2021]   [Dec]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] livepatch: Fix leak on klp_init_patch_early failure path
On Thu 2021-12-16 07:14:27, David Vernet wrote:
> Petr Mladek <pmladek@suse.com> wrote on Thu [2021-Dec-16 15:14:38 +0100]:
> > > Now it does. In the past, I think we did create some memory. I know
> > > when we hook debugobjects up to kobjects (there's an external patch for
> > > that floating around somewhere), that is one reason to keep the
> > > kobject_put() rule, and there might have been other reasons in the past
> > > 20+ years as well.
> > >
> > > So yes, while you are correct today, the "normal" reference counted
> > > object model patern is "after the object is initialized, it MUST only be
> > > freed by handling its reference count." So let's stick to that rule for
> > > now.
> >
> > Good point.
>
> Thanks for the discussion all. I think we've landed on the fact that this
> is a refcounting bug that needs to be fixed, but isn't a leak in how the
> kobject implementation exists today.
>
> Petr - are you OK with me sending out a v3 of the patch with the following
> changes:
> - The patch description is updated to not claim that a leak is being
> fixed, but rather that a kobject reference counting bug is being fixed.
> - All of the NULL checking in klp_init_patch_early() is brought into
> klp_enable_patch(), and klp_init_patch_early() is updated to be void,
> per Josh's suggestion. This would address the refcounting issue and IMO
> also simplifies and improves the code. I know you were onboard with
> moving try_module_get() into klp_enable_patch(), but I don't think we
> ever resolved the point about moving the rest of the NULL checking out
> as well.

Just for record. I have answered this in the other thread where it was
discussed, see https://lore.kernel.org/r/YbyV7nsLXbQ6/44S@alley

Best Regards,
Petr

\
 
 \ /
  Last update: 2021-12-17 14:54    [W:3.104 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site