linux-kernel - Re: [PATCH] genirq/irqdesc: hide illegible sysfs warning of kobject

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y29gXLqK2wiu7/WK@kroah.com>
Date:   Sat, 12 Nov 2022 09:59:08 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Liu Shixin <liushixin2@...wei.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] genirq/irqdesc: hide illegible sysfs warning of
 kobject_del()

On Sat, Nov 12, 2022 at 05:39:39PM +0800, Liu Shixin wrote:
> If irq_sysfs_add() failed, system will report a warning but don't call
> kobject_put() to release the descriptor.

I can not parse this sentance :(

> Then in irq_sysfs_del(), we continue to call kobject_del(). In such
> situation, kobject_del() will complains about a object with no parent
> like this:

Then we should not be calling irq_sysfs_del() if the call failed.  That
is the real fix here.

> 
>  kernfs: can not remove 'actions', no directory
>  WARNING: CPU: 0 PID: 277 at fs/kernfs/dir.c:1615 kernfs_remove_by_name_ns+0xd5/0xe0
> [...]
>  Call Trace:
>   <TASK>
>   remove_files.isra.0+0x3f/0xb0
>   sysfs_remove_group+0x68/0xe0
>   sysfs_remove_groups+0x41/0x70
>   __kobject_del+0x45/0xc0
>   kobject_del+0x2a/0x40
>   free_desc+0x44/0x70
>   irq_free_descs+0x5d/0x90
> [...]
> 
> Use kobj->state_in_sysfs to check whether kobject is added succeed. And
> if not, we should not call kobject_del().

That does not describe what you are doing here at all.

> 
> Signed-off-by: Liu Shixin <liushixin2@...wei.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 a91f9001103c..a820d96210d4 100644
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -300,10 +300,11 @@ static void irq_sysfs_del(struct irq_desc *desc)
>  	/*
>  	 * If irq_sysfs_init() has not yet been invoked (early boot), then
>  	 * irq_kobj_base is NULL and the descriptor was never added.
> +	 * And the descriptor may be added failed.
>  	 * kobject_del() complains about a object with no parent, so make
>  	 * it conditional.
>  	 */
> -	if (irq_kobj_base)
> +	if (irq_kobj_base && desc->kobj.parent)

How would the parent be NULL?  Parent devices always stick around until
the child is removed, otherwise something is really wrong here.  You
should never have to look at the parent.

thanks,

greg k-h