Re: [PATCH] livepatch: Fix leak on klp_init_patch_early failure path

Petr Mladek [off-list ref] wrote on Thu [2021-Dec-16 15:14:38 +0100]:

quoted

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.

Regards,
David