Messages in this thread | | | Date | Wed, 15 Dec 2021 07:20:04 -0800 | From | David Vernet <> | Subject | Re: [PATCH v2] livepatch: Fix leak on klp_init_patch_early failure path |
| |
Petr Mladek <pmladek@suse.com> wrote on Wed [2021-Dec-15 11:06:15 +0100]: > Well, I still believe that this is just a cargo cult. And I would prefer > to finish the discussion about it, first, see > https://lore.kernel.org/all/YbmlL0ZyfSuek9OB@alley/
No problem, I won't send out v3 until we've finished the discussion and have consensus. I'll assume that the discussion on whether or not there is a leak will continue on the thread you linked to above, so I won't comment on it here.
> Note that klp_init_*_early() functions iterate through the arrays > using klp_for_each_*_static. While klp_free_*() functions iterate > via the lists using klp_for_each_*_safe().
Correct, as I've understood it, klp_for_each_*_safe() should only iterate over the objects that have been added to the patch and klp_object's lists, and thus for which kobject_init() has been invoked. So if we fail a check on 'struct klp_object' N, then we'll only iterate over the first N - 1 objects in klp_for_each_*_safe().
> We should not need the pre-early-init check when the lists include only > structures with initialized kobjects.
Not sure I quite follow. We have to do NULL checks for obj->funcs at some point, and per Josh's suggestion it seems cleaner to do it outside the critical section, and before we actually invoke kobject_init(). Apologies if I've misunderstood your point.
> Otherwise, I like the idea to do module_get() before > klp_init_patch_early(). I was never happy with the "hidden" > side effect.
Ack!
> I am also fine with calling klp_free() when the early init fails > if we agreed that it is a good practice. I just do want to pretend > that it fixes a leak what nobody sees any leak. > > Please, wait few days until the discussion finishes before sending v3.
Ack, no problem, I'll wait until we're all in alignment. Thanks, Petr and Josh for taking a look at the patch.
Regards, David
| |