On Thu 2021-12-16 07:14:27, David Vernet wrote: > Petr Mladek <pmladek@xxxxxxxx> 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
