* Re: [PATCH 1/1] genirq: Properly pair kobject_del with kobject_add [not found] <1564703564-4116-1-git-send-email-mikelley@microsoft.com> @ 2019-08-02 6:34 ` gregkh [not found] ` <MWHPR21MB078463AB854A336842118405D7D90@MWHPR21MB0784.namprd21.prod.outlook.com> 2019-08-19 14:01 ` [tip:irq/urgent] genirq: Properly pair kobject_del() with kobject_add() tip-bot for Michael Kelley 1 sibling, 1 reply; 5+ messages in thread From: gregkh @ 2019-08-02 6:34 UTC (permalink / raw) To: Michael Kelley; +Cc: linux-kernel, tglx, boqun.feng, kimbrownkd On Thu, Aug 01, 2019 at 11:53:53PM +0000, Michael Kelley wrote: > If alloc_descs fails before irq_sysfs_init has run, free_desc in the > cleanup path will call kobject_del even though the kobject has not > been added with kobject_add. Fix this by making the call to > kobject_del conditional on whether irq_sysfs_init has run. > > This problem surfaced because commit aa30f47cf666 > ("kobject: Add support for default attribute groups to kobj_type") > makes kobject_del stricter about pairing with kobject_add. If the > pairing is incorrrect, a WARNING and backtrace occur in > sysfs_remove_group because there is no parent. > > Fixes: ecb3f394c5db ("genirq: Expose interrupt information through sysfs") > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > --- > kernel/irq/irqdesc.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c > index 9484e88..5447760 100644 > --- a/kernel/irq/irqdesc.c > +++ b/kernel/irq/irqdesc.c > @@ -438,7 +438,8 @@ static void free_desc(unsigned int irq) > * The sysfs entry must be serialized against a concurrent > * irq_sysfs_init() as well. > */ > - kobject_del(&desc->kobj); > + if (irq_kobj_base) > + kobject_del(&desc->kobj); But now you leak the memory of desc as there is no chance it could be freed, because the kobject release function is never called :( Relying on irq_kobj_bas to be present or not seems like an odd test here. thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <MWHPR21MB078463AB854A336842118405D7D90@MWHPR21MB0784.namprd21.prod.outlook.com>]
* Re: [PATCH 1/1] genirq: Properly pair kobject_del with kobject_add [not found] ` <MWHPR21MB078463AB854A336842118405D7D90@MWHPR21MB0784.namprd21.prod.outlook.com> @ 2019-08-03 7:15 ` gregkh 2019-08-03 7:43 ` Thomas Gleixner 0 siblings, 1 reply; 5+ messages in thread From: gregkh @ 2019-08-03 7:15 UTC (permalink / raw) To: Michael Kelley; +Cc: linux-kernel, tglx, boqun.feng, kimbrownkd On Fri, Aug 02, 2019 at 08:19:37PM +0000, Michael Kelley wrote: > From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org> Sent: Thursday, August 1, 2019 11:34 PM > > On Thu, Aug 01, 2019 at 11:53:53PM +0000, Michael Kelley wrote: > > > If alloc_descs fails before irq_sysfs_init has run, free_desc in the > > > cleanup path will call kobject_del even though the kobject has not > > > been added with kobject_add. Fix this by making the call to > > > kobject_del conditional on whether irq_sysfs_init has run. > > > > > > This problem surfaced because commit aa30f47cf666 > > > ("kobject: Add support for default attribute groups to kobj_type") > > > makes kobject_del stricter about pairing with kobject_add. If the > > > pairing is incorrrect, a WARNING and backtrace occur in > > > sysfs_remove_group because there is no parent. > > > > > > Fixes: ecb3f394c5db ("genirq: Expose interrupt information through sysfs") > > > Signed-off-by: Michael Kelley <mikelley@microsoft.com> > > > --- > > > kernel/irq/irqdesc.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c > > > index 9484e88..5447760 100644 > > > --- a/kernel/irq/irqdesc.c > > > +++ b/kernel/irq/irqdesc.c > > > @@ -438,7 +438,8 @@ static void free_desc(unsigned int irq) > > > * The sysfs entry must be serialized against a concurrent > > > * irq_sysfs_init() as well. > > > */ > > > - kobject_del(&desc->kobj); > > > + if (irq_kobj_base) > > > + kobject_del(&desc->kobj); > > > > But now you leak the memory of desc as there is no chance it could be > > freed, because the kobject release function is never called :( > > In the alloc_descs error path, when irq_kobj_base is still NULL, the > kobject code sequence is: > kobject_init() [as called by alloc_desc] > kobject_put() [as called by delayed_free_desc] > > So I don't think anything leaks. > > If irq_kobj_base is not NULL, the kobject code sequence is: > kobject_init() [as called by alloc_desc] > kobject_add() [as called by irq_sysfs_add] > kobject_del() [as called by free_desc] > kobject_put() [as called by delayed_free_desc] > > Again, everything is paired up properly. > > > > > Relying on irq_kobj_base to be present or not seems like an odd test > > here. > > > > It's the same test that is used in irq_sysfs_add to decide whether to > call kobject_add. So it makes everything paired up and symmetrical. Ugh, that's a tangled mess and totally not obvious at all. I'm sure there's a good reason for all of that, and I really don't want to know :) Anyway, yes, you are right, the patch is fine, sorry for the noise. Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] genirq: Properly pair kobject_del with kobject_add 2019-08-03 7:15 ` gregkh @ 2019-08-03 7:43 ` Thomas Gleixner 0 siblings, 0 replies; 5+ messages in thread From: Thomas Gleixner @ 2019-08-03 7:43 UTC (permalink / raw) To: gregkh; +Cc: Michael Kelley, linux-kernel, boqun.feng, kimbrownkd On Sat, 3 Aug 2019, gregkh@linuxfoundation.org wrote: > On Fri, Aug 02, 2019 at 08:19:37PM +0000, Michael Kelley wrote: > > > Relying on irq_kobj_base to be present or not seems like an odd test > > > here. > > > > > > > It's the same test that is used in irq_sysfs_add to decide whether to > > call kobject_add. So it makes everything paired up and symmetrical. > > Ugh, that's a tangled mess and totally not obvious at all. I'm sure > there's a good reason for all of that, and I really don't want to know > :) It's all your fault that the sysfs stuff does not work in very early boot. /me runs ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:irq/urgent] genirq: Properly pair kobject_del() with kobject_add() [not found] <1564703564-4116-1-git-send-email-mikelley@microsoft.com> 2019-08-02 6:34 ` [PATCH 1/1] genirq: Properly pair kobject_del with kobject_add gregkh @ 2019-08-19 14:01 ` tip-bot for Michael Kelley [not found] ` <20190819212758.6D03D22CEC@mail.kernel.org> 1 sibling, 1 reply; 5+ messages in thread From: tip-bot for Michael Kelley @ 2019-08-19 14:01 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, mikelley, gregkh, hpa, mingo, tglx Commit-ID: e1ee29624746fbf667f80e8ae3815a76e4d1bd5b Gitweb: https://git.kernel.org/tip/e1ee29624746fbf667f80e8ae3815a76e4d1bd5b Author: Michael Kelley <mikelley@microsoft.com> AuthorDate: Thu, 1 Aug 2019 23:53:53 +0000 Committer: Thomas Gleixner <tglx@linutronix.de> CommitDate: Mon, 19 Aug 2019 15:56:28 +0200 genirq: Properly pair kobject_del() with kobject_add() If alloc_descs() fails before irq_sysfs_init() has run, free_desc() in the cleanup path will call kobject_del() even though the kobject has not been added with kobject_add(). Fix this by making the call to kobject_del() conditional on whether irq_sysfs_init() has run. This problem surfaced because commit aa30f47cf666 ("kobject: Add support for default attribute groups to kobj_type") makes kobject_del() stricter about pairing with kobject_add(). If the pairing is incorrrect, a WARNING and backtrace occur in sysfs_remove_group() because there is no parent. [ tglx: Add a comment to the code ] Fixes: ecb3f394c5db ("genirq: Expose interrupt information through sysfs") Signed-off-by: Michael Kelley <mikelley@microsoft.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/1564703564-4116-1-git-send-email-mikelley@microsoft.com --- kernel/irq/irqdesc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 9484e88dabc2..51f42f3caf09 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -437,8 +437,14 @@ static void free_desc(unsigned int irq) * * The sysfs entry must be serialized against a concurrent * irq_sysfs_init() as well. + * + * If irq_sysfs_init() has not yet been invoked (early boot), then + * irq_kobj_base is NULL and the descriptor was never added. + * kobject_del() complains about a object with no parent, so make + * it conditional. */ - kobject_del(&desc->kobj); + if (irq_kobj_base) + kobject_del(&desc->kobj); delete_irq_desc(irq); /* ^ permalink raw reply related [flat|nested] 5+ messages in thread
[parent not found: <20190819212758.6D03D22CEC@mail.kernel.org>]
* RE: [tip:irq/urgent] genirq: Properly pair kobject_del() with kobject_add() [not found] ` <20190819212758.6D03D22CEC@mail.kernel.org> @ 2019-08-19 21:58 ` Michael Kelley 0 siblings, 0 replies; 5+ messages in thread From: Michael Kelley @ 2019-08-19 21:58 UTC (permalink / raw) To: Sasha Levin, tip-bot for Michael Kelley, linux-tip-commits Cc: linux-kernel, stable, stable From: Sasha Levin <sashal@kernel.org> Sent: Monday, August 19, 2019 2:28 PM > > This commit has been processed because it contains a "Fixes:" tag, > fixing commit: ecb3f394c5db genirq: Expose interrupt information through sysfs. > > The bot has tested the following trees: v5.2.9, v4.19.67, v4.14.139, v4.9.189. > > v5.2.9: Build failed! Errors: > kernel/irq/irqdesc.c:446:6: error: ‘irq_kobj_base’ undeclared (first use in this function); > did you mean ‘irq_kobj_type’? > > v4.19.67: Build failed! Errors: > kernel/irq/irqdesc.c:445:6: error: ‘irq_kobj_base’ undeclared (first use in this function); > did you mean ‘irq_kobj_type’? > > v4.14.139: Build failed! Errors: > kernel/irq/irqdesc.c:428:6: error: ‘irq_kobj_base’ undeclared (first use in this function); > did you mean ‘irq_kobj_type’? > > v4.9.189: Build failed! Errors: > kernel/irq/irqdesc.c:414:6: error: ‘irq_kobj_base’ undeclared (first use in this function); > did you mean ‘irq_kobj_type’? > > > NOTE: The patch will not be queued to stable trees until it is upstream. > > How should we proceed with this patch? Compile error occurs when CONFIG_SYSFS is not selected. It's probably cleanest to revert the current patch. I'll send out a new version that fixes the problem. Michael ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-19 21:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1564703564-4116-1-git-send-email-mikelley@microsoft.com> 2019-08-02 6:34 ` [PATCH 1/1] genirq: Properly pair kobject_del with kobject_add gregkh [not found] ` <MWHPR21MB078463AB854A336842118405D7D90@MWHPR21MB0784.namprd21.prod.outlook.com> 2019-08-03 7:15 ` gregkh 2019-08-03 7:43 ` Thomas Gleixner 2019-08-19 14:01 ` [tip:irq/urgent] genirq: Properly pair kobject_del() with kobject_add() tip-bot for Michael Kelley [not found] ` <20190819212758.6D03D22CEC@mail.kernel.org> 2019-08-19 21:58 ` Michael Kelley
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.