Ah, found the thread... On Tue, Dec 14, 2021 at 09:45:53AM +0100, Petr Mladek wrote: > On Mon 2021-12-13 11:17:35, David Vernet wrote: > > When enabling a KLP patch with `klp_enable_patch`, we invoke > > `klp_init_patch_early` to initialize the kobjects for the patch itself, as > > well as the `struct klp_object*`'s and `struct klp_func*`'s that comprise > > it. However, there are some paths where we may fail to do an > > early-initialization of an object or its functions if certain conditions > > are not met, such as an object having a `NULL` funcs pointer. In these > > paths, we may currently leak the `struct klp_patch*`'s kobject, as well as > > any of its objects or functions, as we don't free the patch in > > `klp_enable_patch` if `klp_init_patch_early` returns an error code. > > Could you please explain what exactly are we leaking? > > I do not see anything allocated in klp_init_*_early() functions. > Also I do not see anything allocated in kobject_init(). > > Documentation/core-api/kobject.rst says that kobject_put() must be > used after calling kobject_add(): > > "Once you registered your kobject via kobject_add(), you must never use > kfree() to free it directly. The only safe way is to use kobject_put(). It > is good practice to always use kobject_put() after kobject_init() to avoid > errors creeping in." > > > Hmm, the comment in lib/kobject.c says something else: > > /** > * kobject_init() - Initialize a kobject structure. > * @kobj: pointer to the kobject to initialize > * @ktype: pointer to the ktype for this kobject. > * > * This function will properly initialize a kobject such that it can then > * be passed to the kobject_add() call. > * > * After this function is called, the kobject MUST be cleaned up by a call > * to kobject_put(), not by a call to kfree directly to ensure that all of > * the memory is cleaned up properly. > */ These say the same thing as "good practice" == "MUST" here. You can NOT call kfree after calling kobject_init(). Bad things will happen if you try to do so. > I believe that this comment is misleading. IMHO, kobject_init() allows > to call kobject_put(). You are FORCED TO call kobject_put() after kobject_init() is called. Anything else is a bug. > And it might be used to free memory that has > already been allocated when initializing the structure where this > kobject is bundled. But simple free() is perfectly fine when nothing > else was allocated. Nope, sorry, you have to call kobject_put(). thanks, greg k-h
