* [PATCH 0/2] livepatch: Fix usage of kobject_init_and_add() @ 2019-04-30 0:15 Tobin C. Harding 2019-04-30 0:15 ` [PATCH 1/2] livepatch: Fix kobject memleak Tobin C. Harding 2019-04-30 0:15 ` [PATCH 2/2] livepatch: Use correct kobject cleanup function Tobin C. Harding 0 siblings, 2 replies; 14+ messages in thread From: Tobin C. Harding @ 2019-04-30 0:15 UTC (permalink / raw) To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek Cc: Tobin C. Harding, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel Hi, Currently there are a few places in kernel/livepatch/ which do not correctly use kobject_init_and_add(). An error return from kobject_init_and_add() requires a call to kobject_put(). The cleanup function after a successful call to kobject_init_and_add() is kobject_del(). This set is part of an effort to check/fix all callsites of kobject_init_and_add(). This set fixes all callsites under kernel/livepatch/ thanks, Tobin. Tobin C. Harding (2): livepatch: Fix kobject memleak livepatch: Use correct kobject cleanup function kernel/livepatch/core.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] livepatch: Fix kobject memleak 2019-04-30 0:15 [PATCH 0/2] livepatch: Fix usage of kobject_init_and_add() Tobin C. Harding @ 2019-04-30 0:15 ` Tobin C. Harding 2019-04-30 8:42 ` Greg Kroah-Hartman 2019-04-30 14:56 ` Petr Mladek 2019-04-30 0:15 ` [PATCH 2/2] livepatch: Use correct kobject cleanup function Tobin C. Harding 1 sibling, 2 replies; 14+ messages in thread From: Tobin C. Harding @ 2019-04-30 0:15 UTC (permalink / raw) To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek Cc: Tobin C. Harding, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel Currently error return from kobject_init_and_add() is not followed by a call to kobject_put(). This means there is a memory leak. Add call to kobject_put() in error path of kobject_init_and_add(). Signed-off-by: Tobin C. Harding <tobin@kernel.org> --- kernel/livepatch/core.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index eb0ee10a1981..98a7bec41faa 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -727,7 +727,9 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func) ret = kobject_init_and_add(&func->kobj, &klp_ktype_func, &obj->kobj, "%s,%lu", func->old_name, func->old_sympos ? func->old_sympos : 1); - if (!ret) + if (ret) + kobject_put(&func->kobj); + else func->kobj_added = true; return ret; @@ -803,8 +805,10 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj) name = klp_is_module(obj) ? obj->name : "vmlinux"; ret = kobject_init_and_add(&obj->kobj, &klp_ktype_object, &patch->kobj, "%s", name); - if (ret) + if (ret) { + kobject_put(&func->kobj); return ret; + } obj->kobj_added = true; klp_for_each_func(obj, func) { @@ -862,8 +866,10 @@ static int klp_init_patch(struct klp_patch *patch) ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch, klp_root_kobj, "%s", patch->mod->name); - if (ret) + if (ret) { + kobject_put(&func->kobj); return ret; + } patch->kobj_added = true; if (patch->replace) { -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] livepatch: Fix kobject memleak 2019-04-30 0:15 ` [PATCH 1/2] livepatch: Fix kobject memleak Tobin C. Harding @ 2019-04-30 8:42 ` Greg Kroah-Hartman 2019-04-30 10:44 ` Miroslav Benes 2019-04-30 14:56 ` Petr Mladek 1 sibling, 1 reply; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-04-30 8:42 UTC (permalink / raw) To: Tobin C. Harding Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, live-patching, linux-kernel On Tue, Apr 30, 2019 at 10:15:33AM +1000, Tobin C. Harding wrote: > Currently error return from kobject_init_and_add() is not followed by a > call to kobject_put(). This means there is a memory leak. > > Add call to kobject_put() in error path of kobject_init_and_add(). > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] livepatch: Fix kobject memleak 2019-04-30 8:42 ` Greg Kroah-Hartman @ 2019-04-30 10:44 ` Miroslav Benes 2019-04-30 10:46 ` Greg Kroah-Hartman 2019-04-30 22:39 ` Tobin C. Harding 0 siblings, 2 replies; 14+ messages in thread From: Miroslav Benes @ 2019-04-30 10:44 UTC (permalink / raw) To: Tobin C. Harding, Greg Kroah-Hartman Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence, live-patching, linux-kernel On Tue, 30 Apr 2019, Greg Kroah-Hartman wrote: > On Tue, Apr 30, 2019 at 10:15:33AM +1000, Tobin C. Harding wrote: > > Currently error return from kobject_init_and_add() is not followed by a > > call to kobject_put(). This means there is a memory leak. > > > > Add call to kobject_put() in error path of kobject_init_and_add(). > > > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Well, it does not even compile... On Tue, 30 Apr 2019, Tobin C. Harding wrote: > Currently error return from kobject_init_and_add() is not followed by a > call to kobject_put(). This means there is a memory leak. > > Add call to kobject_put() in error path of kobject_init_and_add(). > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > --- > kernel/livepatch/core.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index eb0ee10a1981..98a7bec41faa 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -727,7 +727,9 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func) > ret = kobject_init_and_add(&func->kobj, &klp_ktype_func, > &obj->kobj, "%s,%lu", func->old_name, > func->old_sympos ? func->old_sympos : 1); > - if (!ret) > + if (ret) > + kobject_put(&func->kobj); > + else > func->kobj_added = true; > > return ret; > @@ -803,8 +805,10 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj) > name = klp_is_module(obj) ? obj->name : "vmlinux"; > ret = kobject_init_and_add(&obj->kobj, &klp_ktype_object, > &patch->kobj, "%s", name); > - if (ret) > + if (ret) { > + kobject_put(&func->kobj); kobject_put(&obj->kobj); I suppose. > return ret; > + } > obj->kobj_added = true; > > klp_for_each_func(obj, func) { > @@ -862,8 +866,10 @@ static int klp_init_patch(struct klp_patch *patch) > > ret = kobject_init_and_add(&patch->kobj, &klp_ktype_patch, > klp_root_kobj, "%s", patch->mod->name); > - if (ret) > + if (ret) { > + kobject_put(&func->kobj); kobject_put(&patch->kobj); Thanks, Miroslav ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] livepatch: Fix kobject memleak 2019-04-30 10:44 ` Miroslav Benes @ 2019-04-30 10:46 ` Greg Kroah-Hartman 2019-04-30 22:39 ` Tobin C. Harding 1 sibling, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-04-30 10:46 UTC (permalink / raw) To: Miroslav Benes Cc: Tobin C. Harding, Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence, live-patching, linux-kernel On Tue, Apr 30, 2019 at 12:44:55PM +0200, Miroslav Benes wrote: > On Tue, 30 Apr 2019, Greg Kroah-Hartman wrote: > > > On Tue, Apr 30, 2019 at 10:15:33AM +1000, Tobin C. Harding wrote: > > > Currently error return from kobject_init_and_add() is not followed by a > > > call to kobject_put(). This means there is a memory leak. > > > > > > Add call to kobject_put() in error path of kobject_init_and_add(). > > > > > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > > > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Well, it does not even compile... The idea is correct :) > On Tue, 30 Apr 2019, Tobin C. Harding wrote: > > > Currently error return from kobject_init_and_add() is not followed by a > > call to kobject_put(). This means there is a memory leak. > > > > Add call to kobject_put() in error path of kobject_init_and_add(). > > > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > > --- > > kernel/livepatch/core.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > index eb0ee10a1981..98a7bec41faa 100644 > > --- a/kernel/livepatch/core.c > > +++ b/kernel/livepatch/core.c > > @@ -727,7 +727,9 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func) > > ret = kobject_init_and_add(&func->kobj, &klp_ktype_func, > > &obj->kobj, "%s,%lu", func->old_name, > > func->old_sympos ? func->old_sympos : 1); > > - if (!ret) > > + if (ret) > > + kobject_put(&func->kobj); > > + else > > func->kobj_added = true; > > > > return ret; > > @@ -803,8 +805,10 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj) > > name = klp_is_module(obj) ? obj->name : "vmlinux"; > > ret = kobject_init_and_add(&obj->kobj, &klp_ktype_object, > > &patch->kobj, "%s", name); > > - if (ret) > > + if (ret) { > > + kobject_put(&func->kobj); > > kobject_put(&obj->kobj); I suppose. Yeah, that makes it better, sorry for not catching that in the review :( greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] livepatch: Fix kobject memleak 2019-04-30 10:44 ` Miroslav Benes 2019-04-30 10:46 ` Greg Kroah-Hartman @ 2019-04-30 22:39 ` Tobin C. Harding 1 sibling, 0 replies; 14+ messages in thread From: Tobin C. Harding @ 2019-04-30 22:39 UTC (permalink / raw) To: Miroslav Benes Cc: Tobin C. Harding, Greg Kroah-Hartman, Josh Poimboeuf, Jiri Kosina, Petr Mladek, Joe Lawrence, live-patching, linux-kernel On Tue, Apr 30, 2019 at 12:44:55PM +0200, Miroslav Benes wrote: > On Tue, 30 Apr 2019, Greg Kroah-Hartman wrote: > > > On Tue, Apr 30, 2019 at 10:15:33AM +1000, Tobin C. Harding wrote: > > > Currently error return from kobject_init_and_add() is not followed by a > > > call to kobject_put(). This means there is a memory leak. > > > > > > Add call to kobject_put() in error path of kobject_init_and_add(). > > > > > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > > > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Well, it does not even compile... My apologies, I did compile this but obviously I don't know how to configure the kernel. Thanks for the review. Tobin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] livepatch: Fix kobject memleak 2019-04-30 0:15 ` [PATCH 1/2] livepatch: Fix kobject memleak Tobin C. Harding 2019-04-30 8:42 ` Greg Kroah-Hartman @ 2019-04-30 14:56 ` Petr Mladek 2019-04-30 15:10 ` Greg Kroah-Hartman 1 sibling, 1 reply; 14+ messages in thread From: Petr Mladek @ 2019-04-30 14:56 UTC (permalink / raw) To: Tobin C. Harding Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel On Tue 2019-04-30 10:15:33, Tobin C. Harding wrote: > Currently error return from kobject_init_and_add() is not followed by a > call to kobject_put(). This means there is a memory leak. I see, the ref count is always initialized to 1 via: + kobject_init_and_add() + kobject_init() + kobject_init_internal() + kref_init() > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > --- > kernel/livepatch/core.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index eb0ee10a1981..98a7bec41faa 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -727,7 +727,9 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func) > ret = kobject_init_and_add(&func->kobj, &klp_ktype_func, > &obj->kobj, "%s,%lu", func->old_name, > func->old_sympos ? func->old_sympos : 1); > - if (!ret) > + if (ret) > + kobject_put(&func->kobj); > + else > func->kobj_added = true; We could actually get rid of the custom kobj_added. Intead, we could check for kobj->state_initialized in the various klp_free* functions. Best Regards, Petr ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] livepatch: Fix kobject memleak 2019-04-30 14:56 ` Petr Mladek @ 2019-04-30 15:10 ` Greg Kroah-Hartman 0 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-04-30 15:10 UTC (permalink / raw) To: Petr Mladek Cc: Tobin C. Harding, Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Joe Lawrence, live-patching, linux-kernel On Tue, Apr 30, 2019 at 04:56:13PM +0200, Petr Mladek wrote: > On Tue 2019-04-30 10:15:33, Tobin C. Harding wrote: > > Currently error return from kobject_init_and_add() is not followed by a > > call to kobject_put(). This means there is a memory leak. > > I see, the ref count is always initialized to 1 via: > > + kobject_init_and_add() > + kobject_init() > + kobject_init_internal() > + kref_init() > > > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > > --- > > kernel/livepatch/core.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > index eb0ee10a1981..98a7bec41faa 100644 > > --- a/kernel/livepatch/core.c > > +++ b/kernel/livepatch/core.c > > @@ -727,7 +727,9 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func) > > ret = kobject_init_and_add(&func->kobj, &klp_ktype_func, > > &obj->kobj, "%s,%lu", func->old_name, > > func->old_sympos ? func->old_sympos : 1); > > - if (!ret) > > + if (ret) > > + kobject_put(&func->kobj); > > + else > > func->kobj_added = true; > > We could actually get rid of the custom kobj_added. Intead, we could > check for kobj->state_initialized in the various klp_free* functions. Why do you need to care about this at all anyway? The kobject can handle it's own lifetime just fine (that's what it is there for), why do you need to also track it? thanks, greg k-h ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] livepatch: Use correct kobject cleanup function 2019-04-30 0:15 [PATCH 0/2] livepatch: Fix usage of kobject_init_and_add() Tobin C. Harding 2019-04-30 0:15 ` [PATCH 1/2] livepatch: Fix kobject memleak Tobin C. Harding @ 2019-04-30 0:15 ` Tobin C. Harding 2019-04-30 8:43 ` Greg Kroah-Hartman ` (2 more replies) 1 sibling, 3 replies; 14+ messages in thread From: Tobin C. Harding @ 2019-04-30 0:15 UTC (permalink / raw) To: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek Cc: Tobin C. Harding, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel The correct cleanup function after a call to kobject_init_and_add() has succeeded is kobject_del() _not_ kobject_put(). kobject_del() calls kobject_put(). Use correct cleanup function when removing a kobject. Signed-off-by: Tobin C. Harding <tobin@kernel.org> --- kernel/livepatch/core.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 98a7bec41faa..4cce6bb6e073 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -589,9 +589,8 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only) list_del(&func->node); - /* Might be called from klp_init_patch() error path. */ if (func->kobj_added) { - kobject_put(&func->kobj); + kobject_del(&func->kobj); } else if (func->nop) { klp_free_func_nop(func); } @@ -625,9 +624,8 @@ static void __klp_free_objects(struct klp_patch *patch, bool nops_only) list_del(&obj->node); - /* Might be called from klp_init_patch() error path. */ if (obj->kobj_added) { - kobject_put(&obj->kobj); + kobject_del(&obj->kobj); } else if (obj->dynamic) { klp_free_object_dynamic(obj); } @@ -676,7 +674,7 @@ static void klp_free_patch_finish(struct klp_patch *patch) * cannot get enabled again. */ if (patch->kobj_added) { - kobject_put(&patch->kobj); + kobject_del(&patch->kobj); wait_for_completion(&patch->finish); } -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] livepatch: Use correct kobject cleanup function 2019-04-30 0:15 ` [PATCH 2/2] livepatch: Use correct kobject cleanup function Tobin C. Harding @ 2019-04-30 8:43 ` Greg Kroah-Hartman 2019-04-30 11:00 ` Miroslav Benes 2019-04-30 15:08 ` Petr Mladek 2 siblings, 0 replies; 14+ messages in thread From: Greg Kroah-Hartman @ 2019-04-30 8:43 UTC (permalink / raw) To: Tobin C. Harding Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Petr Mladek, Joe Lawrence, live-patching, linux-kernel On Tue, Apr 30, 2019 at 10:15:34AM +1000, Tobin C. Harding wrote: > The correct cleanup function after a call to kobject_init_and_add() has > succeeded is kobject_del() _not_ kobject_put(). kobject_del() calls > kobject_put(). > > Use correct cleanup function when removing a kobject. > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] livepatch: Use correct kobject cleanup function 2019-04-30 0:15 ` [PATCH 2/2] livepatch: Use correct kobject cleanup function Tobin C. Harding 2019-04-30 8:43 ` Greg Kroah-Hartman @ 2019-04-30 11:00 ` Miroslav Benes 2019-04-30 21:38 ` Tobin C. Harding 2019-04-30 15:08 ` Petr Mladek 2 siblings, 1 reply; 14+ messages in thread From: Miroslav Benes @ 2019-04-30 11:00 UTC (permalink / raw) To: Tobin C. Harding Cc: Josh Poimboeuf, Jiri Kosina, Petr Mladek, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel On Tue, 30 Apr 2019, Tobin C. Harding wrote: > The correct cleanup function after a call to kobject_init_and_add() has > succeeded is kobject_del() _not_ kobject_put(). kobject_del() calls > kobject_put(). > > Use correct cleanup function when removing a kobject. > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > --- > kernel/livepatch/core.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > index 98a7bec41faa..4cce6bb6e073 100644 > --- a/kernel/livepatch/core.c > +++ b/kernel/livepatch/core.c > @@ -589,9 +589,8 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only) > > list_del(&func->node); > > - /* Might be called from klp_init_patch() error path. */ Could you leave the comment as is? If I am not mistaken, it is still valid. func->kobj_added check is here exactly because the function may be called as mentioned. One could argue that the comment is not so important, but the change does not belong to the patch anyway in my opinion. > if (func->kobj_added) { > - kobject_put(&func->kobj); > + kobject_del(&func->kobj); > } else if (func->nop) { > klp_free_func_nop(func); > } > @@ -625,9 +624,8 @@ static void __klp_free_objects(struct klp_patch *patch, bool nops_only) > > list_del(&obj->node); > > - /* Might be called from klp_init_patch() error path. */ Same here. > if (obj->kobj_added) { > - kobject_put(&obj->kobj); > + kobject_del(&obj->kobj); > } else if (obj->dynamic) { > klp_free_object_dynamic(obj); > } > @@ -676,7 +674,7 @@ static void klp_free_patch_finish(struct klp_patch *patch) > * cannot get enabled again. > */ > if (patch->kobj_added) { > - kobject_put(&patch->kobj); > + kobject_del(&patch->kobj); > wait_for_completion(&patch->finish); > } Miroslav ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] livepatch: Use correct kobject cleanup function 2019-04-30 11:00 ` Miroslav Benes @ 2019-04-30 21:38 ` Tobin C. Harding 0 siblings, 0 replies; 14+ messages in thread From: Tobin C. Harding @ 2019-04-30 21:38 UTC (permalink / raw) To: Miroslav Benes Cc: Tobin C. Harding, Josh Poimboeuf, Jiri Kosina, Petr Mladek, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel On Tue, Apr 30, 2019 at 01:00:05PM +0200, Miroslav Benes wrote: > On Tue, 30 Apr 2019, Tobin C. Harding wrote: > > > The correct cleanup function after a call to kobject_init_and_add() has > > succeeded is kobject_del() _not_ kobject_put(). kobject_del() calls > > kobject_put(). > > > > Use correct cleanup function when removing a kobject. > > > > Signed-off-by: Tobin C. Harding <tobin@kernel.org> > > --- > > kernel/livepatch/core.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > index 98a7bec41faa..4cce6bb6e073 100644 > > --- a/kernel/livepatch/core.c > > +++ b/kernel/livepatch/core.c > > @@ -589,9 +589,8 @@ static void __klp_free_funcs(struct klp_object *obj, bool nops_only) > > > > list_del(&func->node); > > > > - /* Might be called from klp_init_patch() error path. */ > > Could you leave the comment as is? If I am not mistaken, it is still > valid. func->kobj_added check is here exactly because the function may be > called as mentioned. Will put it back in for you on v2 thanks, Tobin. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] livepatch: Use correct kobject cleanup function 2019-04-30 0:15 ` [PATCH 2/2] livepatch: Use correct kobject cleanup function Tobin C. Harding 2019-04-30 8:43 ` Greg Kroah-Hartman 2019-04-30 11:00 ` Miroslav Benes @ 2019-04-30 15:08 ` Petr Mladek 2019-04-30 21:37 ` Tobin C. Harding 2 siblings, 1 reply; 14+ messages in thread From: Petr Mladek @ 2019-04-30 15:08 UTC (permalink / raw) To: Tobin C. Harding Cc: Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel On Tue 2019-04-30 10:15:34, Tobin C. Harding wrote: > The correct cleanup function after a call to kobject_init_and_add() has > succeeded is kobject_del() _not_ kobject_put(). kobject_del() calls > kobject_put(). Really? I see only kobject_put(kobj->parent) in kobject_del. It decreases a reference of the _parent_ object and not the given one. Also the section "Kobject removal" in Documentation/kobject.txt says that kobject_del() is for two-stage removal. kobject_put() still needs to get called at a later time. IMHO, this patch causes that kobject_put() would never get called. That said, we could probably make the removal a bit cleaner by using kobject_del() in klp_free_patch_start() and kobject_put() in klp_free_patch_finish(). But I have to think more about it. Best Regards, Petr ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] livepatch: Use correct kobject cleanup function 2019-04-30 15:08 ` Petr Mladek @ 2019-04-30 21:37 ` Tobin C. Harding 0 siblings, 0 replies; 14+ messages in thread From: Tobin C. Harding @ 2019-04-30 21:37 UTC (permalink / raw) To: Petr Mladek Cc: Tobin C. Harding, Josh Poimboeuf, Jiri Kosina, Miroslav Benes, Greg Kroah-Hartman, Joe Lawrence, live-patching, linux-kernel On Tue, Apr 30, 2019 at 05:08:11PM +0200, Petr Mladek wrote: > On Tue 2019-04-30 10:15:34, Tobin C. Harding wrote: > > The correct cleanup function after a call to kobject_init_and_add() has > > succeeded is kobject_del() _not_ kobject_put(). kobject_del() calls > > kobject_put(). > > Really? I see only kobject_put(kobj->parent) in kobject_del. > It decreases a reference of the _parent_ object and not > the given one. Thanks Petr, you are right. I misread kobject_del(). The story thickens, so we need to call kobject_del() AND kobject_put(). > Also the section "Kobject removal" in Documentation/kobject.txt > says that kobject_del() is for two-stage removal. kobject_put() > still needs to get called at a later time. Is this call sequence above what is meant by 'two-stage removal', I didn't really understand that bit of the docs (and I almost always just assume docs are stale and take them as a hint only :) > IMHO, this patch causes that kobject_put() would never get called. I'll do a v2 of this one and re-check all the patches on this I've already sent (including the docs ones). > That said, we could probably make the removal a bit cleaner > by using kobject_del() in klp_free_patch_start() and > kobject_put() in klp_free_patch_finish(). But I have > to think more about it. Noted, thanks for your review. Tobin ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-04-30 22:40 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-30 0:15 [PATCH 0/2] livepatch: Fix usage of kobject_init_and_add() Tobin C. Harding 2019-04-30 0:15 ` [PATCH 1/2] livepatch: Fix kobject memleak Tobin C. Harding 2019-04-30 8:42 ` Greg Kroah-Hartman 2019-04-30 10:44 ` Miroslav Benes 2019-04-30 10:46 ` Greg Kroah-Hartman 2019-04-30 22:39 ` Tobin C. Harding 2019-04-30 14:56 ` Petr Mladek 2019-04-30 15:10 ` Greg Kroah-Hartman 2019-04-30 0:15 ` [PATCH 2/2] livepatch: Use correct kobject cleanup function Tobin C. Harding 2019-04-30 8:43 ` Greg Kroah-Hartman 2019-04-30 11:00 ` Miroslav Benes 2019-04-30 21:38 ` Tobin C. Harding 2019-04-30 15:08 ` Petr Mladek 2019-04-30 21:37 ` Tobin C. Harding
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.