linux-ext4 - Re: [PATCH 3/3] fs/ext4: create ext4

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]

Date:   Mon, 27 Nov 2017 16:44:35 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     Riccardo Schirone <sirmy15@...il.com>
Cc:     Theodore Ts'o <tytso@....edu>,
        linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH 3/3] fs/ext4: create ext4_kset dynamically

On Nov 27, 2017, at 4:18 PM, Riccardo Schirone <sirmy15@...il.com> wrote:
> 
> ksets contain a kobject and they should always be allocated dynamically,
> because it is unknown to whoever creates them when ksets can be
> released.
> 
> Signed-off-by: Riccardo Schirone <sirmy15@...il.com>
> ---
> fs/ext4/sysfs.c | 37 +++++++++++++++++++++++--------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
> index c447f23cc876..a33b406b8feb 100644
> --- a/fs/ext4/sysfs.c
> +++ b/fs/ext4/sysfs.c
> @@ -329,6 +329,13 @@ static void ext4_sb_release(struct kobject *kobj)
> 	complete(&sbi->s_kobj_unregister);
> }
> 
> +static void ext4_kset_release(struct kobject *kobj)
> +{
> +	struct kset *kset = container_of(kobj, struct kset, kobj);
> +
> +	kfree(kset);
> +}
> +
> static const struct sysfs_ops ext4_attr_ops = {
> 	.show	= ext4_attr_show,
> 	.store	= ext4_attr_store,
> @@ -342,11 +349,10 @@ static struct kobj_type ext4_sb_ktype = {
> 
> static struct kobj_type ext4_ktype = {
> 	.sysfs_ops	= &ext4_attr_ops,
> +	.release	= ext4_kset_release,
> };
> 
> -static struct kset ext4_kset = {
> -	.kobj   = {.ktype = &ext4_ktype},
> -};
> +static struct kset *ext4_kset;
> 
> static struct kobj_type ext4_feat_ktype = {
> 	.default_attrs	= ext4_feat_attrs,
> @@ -391,7 +397,7 @@ int ext4_register_sysfs(struct super_block *sb)
> 	const struct ext4_proc_files *p;
> 	int err;
> 
> -	sbi->s_kobj.kset = &ext4_kset;
> +	sbi->s_kobj.kset = ext4_kset;
> 	init_completion(&sbi->s_kobj_unregister);
> 	err = kobject_init_and_add(&sbi->s_kobj, &ext4_sb_ktype, NULL,
> 				   "%s", sb->s_id);
> @@ -429,13 +435,16 @@ int __init ext4_init_sysfs(void)
> {
> 	int ret;
> 
> -	kobject_set_name(&ext4_kset.kobj, "ext4");
> -	ext4_kset.kobj.parent = fs_kobj;
> -	ret = kset_register(&ext4_kset);
> -	if (ret) {
> -		kset_unregister(&ext4_kset);
> -		return ret;
> -	}
> +	ext4_kset = kzalloc(sizeof(*ext4_kset), GFP_KERNEL);
> +	if (!ext4_kset)
> +		return -ENOMEM;
> +
> +	kobject_set_name(&ext4_kset->kobj, "ext4");
> +	ext4_kset->kobj.parent = fs_kobj;
> +	ext4_kset->kobj.ktype = &ext4_ktype;
> +	ret = kset_register(ext4_kset);
> +	if (ret)
> +		goto kset_err;
> 
> 	ext4_feat = kzalloc(sizeof(*ext4_feat), GFP_KERNEL);
> 	if (!ext4_feat) {
> @@ -443,7 +452,7 @@ int __init ext4_init_sysfs(void)
> 		goto kset_err;
> 	}
> 
> -	ext4_feat->kset = &ext4_kset;
> +	ext4_feat->kset = ext4_kset;
> 	ret = kobject_init_and_add(ext4_feat, &ext4_feat_ktype,
> 				   NULL, "features");
> 	if (ret)
> @@ -455,14 +464,14 @@ int __init ext4_init_sysfs(void)
> feat_err:
> 	kobject_put(ext4_feat);
> kset_err:
> -	kset_unregister(&ext4_kset);
> +	kset_unregister(ext4_kset);


It would be prudent in this case to set "ext4_kset = NULL" here
so that it isn't cleaned up again somewhere else.  Otherwise,
it seems possible that ext4_kset could be cleaned up twice.

Otherwise, the whole premise of this patch seems flawed.

> 	return ret;
> }
> 
> void ext4_exit_sysfs(void)
> {
> 	kobject_put(ext4_feat);
> -	kset_unregister(&ext4_kset);
> +	kset_unregister(ext4_kset);

Same here.

> 	remove_proc_entry(proc_dirname, NULL);
> 	ext4_proc_root = NULL;
> }
> --
> 2.14.3
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)