linux-kernel - Re: [PATCH 3/3] zonefs: convert to use kobject_is

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <2023040616-armory-unmade-4422@gregkh>
Date:   Thu, 6 Apr 2023 12:05:03 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Yangtao Li <frank.li@...o.com>
Cc:     xiang@...nel.org, chao@...nel.org, huyue2@...lpad.com,
        jefflexu@...ux.alibaba.com, damien.lemoal@...nsource.wdc.com,
        naohiro.aota@....com, jth@...nel.org, rafael@...nel.org,
        linux-erofs@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 3/3] zonefs: convert to use kobject_is_added()

On Thu, Apr 06, 2023 at 05:30:56PM +0800, Yangtao Li wrote:
> Use kobject_is_added() instead of local `s_sysfs_registered` variables.
> BTW kill kobject_del() directly, because kobject_put() actually covers
> kobject removal automatically.
> 
> Signed-off-by: Yangtao Li <frank.li@...o.com>
> ---
>  fs/zonefs/sysfs.c  | 11 +++++------
>  fs/zonefs/zonefs.h |  1 -
>  2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/zonefs/sysfs.c b/fs/zonefs/sysfs.c
> index 8ccb65c2b419..f0783bf7a25c 100644
> --- a/fs/zonefs/sysfs.c
> +++ b/fs/zonefs/sysfs.c
> @@ -101,8 +101,6 @@ int zonefs_sysfs_register(struct super_block *sb)
>  		return ret;
>  	}
>  
> -	sbi->s_sysfs_registered = true;

You know this, why do you need to have a variable tell you this or not?

> -
>  	return 0;
>  }
>  
> @@ -110,12 +108,13 @@ void zonefs_sysfs_unregister(struct super_block *sb)
>  {
>  	struct zonefs_sb_info *sbi = ZONEFS_SB(sb);
>  
> -	if (!sbi || !sbi->s_sysfs_registered)

How can either of these ever be true?  Note, sbi should be passed here
to this function, not the super block as that is now unregistered from
the system.  Looks like no one has really tested this codepath that much
:(

> +	if (!sbi)
>  		return;

this can not ever be true, right?


>  
> -	kobject_del(&sbi->s_kobj);
> -	kobject_put(&sbi->s_kobj);
> -	wait_for_completion(&sbi->s_kobj_unregister);
> +	if (kobject_is_added(&sbi->s_kobj)) {

Again, not needed.

thanks,

greg k-h