diff mbox

[CFT,09/10] sysfs: Create mountpoints with sysfs_create_empty_dir

Message ID 878ucrhxi9.fsf@x220.int.ebiederm.org (mailing list archive)
State New, archived
Headers show

Commit Message

Eric W. Biederman May 14, 2015, 5:36 p.m. UTC
This allows for better documentation in the code and
it allows for a simpler and fully correct version of
fs_fully_visible to be written.

The mount points converted and their filesystems are:
/sys/hypervisor/s390/       s390_hypfs
/sys/kernel/config/         configfs
/sys/kernel/debug/          debugfs
/sys/firmware/efi/efivars/  efivarfs
/sys/fs/fuse/connections/   fusectl
/sys/fs/pstore/             pstore
/sys/kernel/tracing/        tracefs
/sys/fs/cgroup/             cgroup
/sys/kernel/security/       securityfs
/sys/fs/selinux/            selinuxfs
/sys/fs/smackfs/            smackfs

Cc: stable@vger.kernel.org
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/s390/hypfs/inode.c      | 12 ++++--------
 drivers/firmware/efi/efi.c   |  6 ++----
 fs/configfs/mount.c          | 10 ++++------
 fs/debugfs/inode.c           | 11 ++++-------
 fs/fuse/inode.c              |  9 +++------
 fs/pstore/inode.c            | 12 ++++--------
 fs/tracefs/inode.c           |  6 ++----
 kernel/cgroup.c              | 10 ++++------
 security/inode.c             | 10 ++++------
 security/selinux/selinuxfs.c | 11 +++++------
 security/smack/smackfs.c     |  8 ++++----
 11 files changed, 40 insertions(+), 65 deletions(-)

Comments

Tejun Heo Aug. 11, 2015, 6:44 p.m. UTC | #1
On Thu, May 14, 2015 at 12:36:30PM -0500, Eric W. Biederman wrote:
> 
> This allows for better documentation in the code and
> it allows for a simpler and fully correct version of
> fs_fully_visible to be written.
> 
> The mount points converted and their filesystems are:
> /sys/hypervisor/s390/       s390_hypfs
> /sys/kernel/config/         configfs
> /sys/kernel/debug/          debugfs
> /sys/firmware/efi/efivars/  efivarfs
> /sys/fs/fuse/connections/   fusectl
> /sys/fs/pstore/             pstore
> /sys/kernel/tracing/        tracefs
> /sys/fs/cgroup/             cgroup
> /sys/kernel/security/       securityfs
> /sys/fs/selinux/            selinuxfs
> /sys/fs/smackfs/            smackfs
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

So, this somehow ends up confusing upstart on centos6 based systems
making it fail to mount tmpfs on /sys/fs/cgroup.  It also skips sunrpc
and other mounts are different too.  No idea why at this point.  Can
we please revert this from -stable until we know what's going on?

Thanks.
Eric W. Biederman Aug. 11, 2015, 6:57 p.m. UTC | #2
Tejun Heo <tj@kernel.org> writes:

> On Thu, May 14, 2015 at 12:36:30PM -0500, Eric W. Biederman wrote:
>> 
>> This allows for better documentation in the code and
>> it allows for a simpler and fully correct version of
>> fs_fully_visible to be written.
>> 
>> The mount points converted and their filesystems are:
>> /sys/hypervisor/s390/       s390_hypfs
>> /sys/kernel/config/         configfs
>> /sys/kernel/debug/          debugfs
>> /sys/firmware/efi/efivars/  efivarfs
>> /sys/fs/fuse/connections/   fusectl
>> /sys/fs/pstore/             pstore
>> /sys/kernel/tracing/        tracefs
>> /sys/fs/cgroup/             cgroup
>> /sys/kernel/security/       securityfs
>> /sys/fs/selinux/            selinuxfs
>> /sys/fs/smackfs/            smackfs
>> 
>> Cc: stable@vger.kernel.org
>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> So, this somehow ends up confusing upstart on centos6 based systems
> making it fail to mount tmpfs on /sys/fs/cgroup.  It also skips sunrpc
> and other mounts are different too.  No idea why at this point.  Can
> we please revert this from -stable until we know what's going on?

*Boggle*

The only time this should prevent anything is when in a container when
you are not global root.  And then only mounting sysfs should be
affected.

The only difference in executed code really should be setting an extra
flag on the kernfs, inode.  The kernfs changes will also refuse to add
entries to these directories (but these directories are empty).

If this is causing problems I don't have a problem with a revert but
reverts take a minute, and this seems to be the first report of this
kind.  Can we take a minute and attempt to get a coherent explanation.

From what little information you given above it sounds like something
shifted and when you rebuilt the kernel and now a memory stomp is
hitting something else.  It should be a matter of moments to debug this
issue (once a test environment is setup), and see what is wrong and then
we can act intelligently.  Tracing a single system call is not difficult.

If there really is some weird issue I want to know what it is.

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Lutomirski Aug. 11, 2015, 7:21 p.m. UTC | #3
On Tue, Aug 11, 2015 at 11:57 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Tejun Heo <tj@kernel.org> writes:
>
>> On Thu, May 14, 2015 at 12:36:30PM -0500, Eric W. Biederman wrote:
>>>
>>> This allows for better documentation in the code and
>>> it allows for a simpler and fully correct version of
>>> fs_fully_visible to be written.
>>>
>>> The mount points converted and their filesystems are:
>>> /sys/hypervisor/s390/       s390_hypfs
>>> /sys/kernel/config/         configfs
>>> /sys/kernel/debug/          debugfs
>>> /sys/firmware/efi/efivars/  efivarfs
>>> /sys/fs/fuse/connections/   fusectl
>>> /sys/fs/pstore/             pstore
>>> /sys/kernel/tracing/        tracefs
>>> /sys/fs/cgroup/             cgroup
>>> /sys/kernel/security/       securityfs
>>> /sys/fs/selinux/            selinuxfs
>>> /sys/fs/smackfs/            smackfs
>>>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
>>
>> So, this somehow ends up confusing upstart on centos6 based systems
>> making it fail to mount tmpfs on /sys/fs/cgroup.  It also skips sunrpc
>> and other mounts are different too.  No idea why at this point.  Can
>> we please revert this from -stable until we know what's going on?
>
> *Boggle*
>
> The only time this should prevent anything is when in a container when
> you are not global root.  And then only mounting sysfs should be
> affected.

Before:

open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
0666) = -1 EACCES (Permission denied)


After:

open("/sys/kernel/debug/asdf", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK,
0666) = -1 ENOENT (No such file or directory)

Something broke.  I don't know whether CentOS cares about that change,
but there could be other odd side effects.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Aug. 11, 2015, 8:11 p.m. UTC | #4
Hey,

On Tue, Aug 11, 2015 at 2:57 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
>> So, this somehow ends up confusing upstart on centos6 based systems
>> making it fail to mount tmpfs on /sys/fs/cgroup.  It also skips sunrpc
>> and other mounts are different too.  No idea why at this point.  Can
>> we please revert this from -stable until we know what's going on?
>
> *Boggle*
>
> The only time this should prevent anything is when in a container when
> you are not global root.  And then only mounting sysfs should be
> affected.

This is just plain boot. No namespace involved.

> The only difference in executed code really should be setting an extra
> flag on the kernfs, inode.  The kernfs changes will also refuse to add
> entries to these directories (but these directories are empty).

Why do we have this in -stable then? Is this part of a larger fix?

> If this is causing problems I don't have a problem with a revert but
> reverts take a minute, and this seems to be the first report of this
> kind.  Can we take a minute and attempt to get a coherent explanation.
>
> From what little information you given above it sounds like something
> shifted and when you rebuilt the kernel and now a memory stomp is
> hitting something else.  It should be a matter of moments to debug this

I don't think it's a random memory stomping thing. I reverted the
commit from two different kernels and the result was always
consistent.

> issue (once a test environment is setup), and see what is wrong and then
> we can act intelligently.  Tracing a single system call is not difficult.

I'm already out today so it'll have to wait till tomorrow.

> If there really is some weird issue I want to know what it is.

Sure, but you wanna do that in -stable?

Thanks.
Eric W. Biederman Aug. 12, 2015, 12:37 a.m. UTC | #5
Tejun Heo <tj@kernel.org> writes:

> Hey,
>
> On Tue, Aug 11, 2015 at 2:57 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>>> So, this somehow ends up confusing upstart on centos6 based systems
>>> making it fail to mount tmpfs on /sys/fs/cgroup.  It also skips sunrpc
>>> and other mounts are different too.  No idea why at this point.  Can
>>> we please revert this from -stable until we know what's going on?
>>
>> *Boggle*
>>
>> The only time this should prevent anything is when in a container when
>> you are not global root.  And then only mounting sysfs should be
>> affected.
>
> This is just plain boot. No namespace involved.
>
>> The only difference in executed code really should be setting an extra
>> flag on the kernfs, inode.  The kernfs changes will also refuse to add
>> entries to these directories (but these directories are empty).
>
> Why do we have this in -stable then? Is this part of a larger fix?

It is. This patch is part of the prep work to prevent unprivileged users
not mounting sysfs (using user namespace permissions) when they should
not be allowed to.

>> If this is causing problems I don't have a problem with a revert but
>> reverts take a minute, and this seems to be the first report of this
>> kind.  Can we take a minute and attempt to get a coherent explanation.
>>
>> It should be a matter of moments to debug this
>> issue (once a test environment is setup), and see what is wrong and then
>> we can act intelligently.  Tracing a single system call is not difficult.
>
> I'm already out today so it'll have to wait till tomorrow.
>
>> If there really is some weird issue I want to know what it is.
>
> Sure, but you wanna do that in -stable?

Before fixing anything I want a bug report that is clear enough
to be reproducible.

I just went and attempted to reproduce this, and on RHEL6 workstation
(aka my work laptop), using the todays 4.2.0-rc6+ aka
edf15b4d4b01b565cb5f4fd2e2d08940b9f92e2f and all of the mounts in
/proc/self/mounts are the same between 4.2.0-rc6 and the RHEL6 stock
2.6.32-504.30.3.el6.x86_64, including the cgroups mounted on /cgroup.

Which means that I don't have any reason to believe that normal CentOS 6
is broken.

Which -stable kernel are you having problems with?  Perhaps it was
a broken backport?

Is it possible this is a local CentOS 6 hack that is breaking?
Perhaps a patch you apply on top of your -stable kernel?

Certainly with cgroups expected to be mounted at /sys/fs/cgroup there
has clearly been at least one change from the stock configuration.

I think it is a little less serious if stock CentOS 6 doesn't have
problems.  Unless it is a conflict of kernel patches I definitely think
whatever it is needs to be fixed.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman Aug. 12, 2015, 3:58 a.m. UTC | #6
ebiederm@xmission.com (Eric W. Biederman) writes:

> I just went and attempted to reproduce this, and on RHEL6 workstation
> (aka my work laptop), using the todays 4.2.0-rc6+ aka
> edf15b4d4b01b565cb5f4fd2e2d08940b9f92e2f and all of the mounts in
> /proc/self/mounts are the same between 4.2.0-rc6 and the RHEL6 stock
> 2.6.32-504.30.3.el6.x86_64, including the cgroups mounted on /cgroup.

I built a few more kernels just to see if this was some weird backport
thing. The kernels 3.10.86, 3.14.58, 3.18.20, and 4.1.5 all boot and
mount their cgroup filesystems just fine.  Granted I kept having to
smack the memory cgroup into being compiled in as the config options
kept changing but otherwise I have not seen any problems.

So I am very surprised you are having problems.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman Aug. 12, 2015, 4:04 a.m. UTC | #7
ebiederm@xmission.com (Eric W. Biederman) writes:

> ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> I just went and attempted to reproduce this, and on RHEL6 workstation
>> (aka my work laptop), using the todays 4.2.0-rc6+ aka
>> edf15b4d4b01b565cb5f4fd2e2d08940b9f92e2f and all of the mounts in
>> /proc/self/mounts are the same between 4.2.0-rc6 and the RHEL6 stock
>> 2.6.32-504.30.3.el6.x86_64, including the cgroups mounted on /cgroup.
>
> I built a few more kernels just to see if this was some weird backport
> thing. The kernels 3.10.86, 3.14.58, 3.18.20, and 4.1.5 all boot and
> mount their cgroup filesystems just fine.  Granted I kept having to
> smack the memory cgroup into being compiled in as the config options
> kept changing but otherwise I have not seen any problems.
>
> So I am very surprised you are having problems.

Although I guess I could have saved myself some time by noticing that
4.1.5 was the only one of the kernels with the change backported into
it.  *Shrug*

I don't see the problem and I don't know where to look to see why you
are having problems.

Eric

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo Aug. 12, 2015, 7:15 p.m. UTC | #8
Hello, Eric.

On Tue, Aug 11, 2015 at 11:04:28PM -0500, Eric W. Biederman wrote:
> ebiederm@xmission.com (Eric W. Biederman) writes:
> 
> > ebiederm@xmission.com (Eric W. Biederman) writes:
> >
> >> I just went and attempted to reproduce this, and on RHEL6 workstation
> >> (aka my work laptop), using the todays 4.2.0-rc6+ aka
> >> edf15b4d4b01b565cb5f4fd2e2d08940b9f92e2f and all of the mounts in
> >> /proc/self/mounts are the same between 4.2.0-rc6 and the RHEL6 stock
> >> 2.6.32-504.30.3.el6.x86_64, including the cgroups mounted on /cgroup.
> >
> > I built a few more kernels just to see if this was some weird backport
> > thing. The kernels 3.10.86, 3.14.58, 3.18.20, and 4.1.5 all boot and
> > mount their cgroup filesystems just fine.  Granted I kept having to
> > smack the memory cgroup into being compiled in as the config options
> > kept changing but otherwise I have not seen any problems.
> >
> > So I am very surprised you are having problems.
> 
> Although I guess I could have saved myself some time by noticing that
> 4.1.5 was the only one of the kernels with the change backported into
> it.  *Shrug*
> 
> I don't see the problem and I don't know where to look to see why you
> are having problems.

lol, this wasn't upstart but an internal tool which sets up a custom
cgroup hierarchy and the problem was the size of the directory inode
reported by stat(2).  It's kinda hilarious but that's what the tool
was depending on to tell whether tmpfs is mounted on /sys/fs/cgroup or
not.  A kernfs directory reports zero as its inode size while tmpfs
reports some non-zero number, so the tool did stat(2) on
/sys/fs/cgroup and mounted tmpfs iff size is zero to avoid mounting
tmpfs multiple times.  Now, make_empty_dir_inode() sets i_size to 2
and the tool thinks that tmpfs is already mounted there.

It's an icky behavior but it'd be better to maintain the original
behavior.  We should be able to set size to zero for empty dirs,
right?

Thanks.
Tejun Heo Aug. 12, 2015, 8:18 p.m. UTC | #9
On Wed, Aug 12, 2015 at 03:07:19PM -0500, Eric W. Biederman wrote:
> 
> Before the make_empty_dir_inode calls were introduce into proc, sysfs,
> and sysctl those directories when stated reported an i_size of 0.
> make_empty_dir_inode started reporting an i_size of 2.  At least one
> userspace application depended on stat returning i_size of 0.  So modify
> make_empty_dir_inode to cause an i_size of 0 to be reported for these
> directories.
> 
> Cc: stable@vger.kernel.org
> Reproted-by: Tejun Heo <tj@kernel.org>
    ^^^
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.
diff mbox

Patch

diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
index d3f896a35b98..d943d36076cc 100644
--- a/arch/s390/hypfs/inode.c
+++ b/arch/s390/hypfs/inode.c
@@ -456,8 +456,6 @@  static const struct super_operations hypfs_s_ops = {
 	.show_options	= hypfs_show_options,
 };
 
-static struct kobject *s390_kobj;
-
 static int __init hypfs_init(void)
 {
 	int rc;
@@ -481,18 +479,16 @@  static int __init hypfs_init(void)
 		rc = -ENODATA;
 		goto fail_hypfs_sprp_exit;
 	}
-	s390_kobj = kobject_create_and_add("s390", hypervisor_kobj);
-	if (!s390_kobj) {
-		rc = -ENOMEM;
+	rc = sysfs_create_empty_dir(hypervisor_kobj, "s390");
+	if (rc)
 		goto fail_hypfs_diag0c_exit;
-	}
 	rc = register_filesystem(&hypfs_type);
 	if (rc)
 		goto fail_filesystem;
 	return 0;
 
 fail_filesystem:
-	kobject_put(s390_kobj);
+	sysfs_remove_empty_dir(hypervisor_kobj, "s390");
 fail_hypfs_diag0c_exit:
 	hypfs_diag0c_exit();
 fail_hypfs_sprp_exit:
@@ -510,7 +506,7 @@  fail_dbfs_exit:
 static void __exit hypfs_exit(void)
 {
 	unregister_filesystem(&hypfs_type);
-	kobject_put(s390_kobj);
+	sysfs_remove_empty_dir(hypervisor_kobj, "s390");
 	hypfs_diag0c_exit();
 	hypfs_sprp_exit();
 	hypfs_vm_exit();
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 3061bb8629dc..98523650efd9 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -65,7 +65,6 @@  static int __init parse_efi_cmdline(char *str)
 early_param("efi", parse_efi_cmdline);
 
 static struct kobject *efi_kobj;
-static struct kobject *efivars_kobj;
 
 /*
  * Let's not leave out systab information that snuck into
@@ -212,10 +211,9 @@  static int __init efisubsys_init(void)
 		goto err_remove_group;
 
 	/* and the standard mountpoint for efivarfs */
-	efivars_kobj = kobject_create_and_add("efivars", efi_kobj);
-	if (!efivars_kobj) {
+	error = sysfs_create_empty_dir(efi_kobj, "efivars");
+	if (error) {
 		pr_err("efivars: Subsystem registration failed.\n");
-		error = -ENOMEM;
 		goto err_remove_group;
 	}
 
diff --git a/fs/configfs/mount.c b/fs/configfs/mount.c
index da94e41bdbf6..b4d1580a6602 100644
--- a/fs/configfs/mount.c
+++ b/fs/configfs/mount.c
@@ -129,8 +129,6 @@  void configfs_release_fs(void)
 }
 
 
-static struct kobject *config_kobj;
-
 static int __init configfs_init(void)
 {
 	int err = -ENOMEM;
@@ -141,8 +139,8 @@  static int __init configfs_init(void)
 	if (!configfs_dir_cachep)
 		goto out;
 
-	config_kobj = kobject_create_and_add("config", kernel_kobj);
-	if (!config_kobj)
+	err = sysfs_create_empty_dir(kernel_kobj, "config");
+	if (err)
 		goto out2;
 
 	err = register_filesystem(&configfs_fs_type);
@@ -152,7 +150,7 @@  static int __init configfs_init(void)
 	return 0;
 out3:
 	pr_err("Unable to register filesystem!\n");
-	kobject_put(config_kobj);
+	sysfs_remove_empty_dir(kernel_kobj, "config");
 out2:
 	kmem_cache_destroy(configfs_dir_cachep);
 	configfs_dir_cachep = NULL;
@@ -163,7 +161,7 @@  out:
 static void __exit configfs_exit(void)
 {
 	unregister_filesystem(&configfs_fs_type);
-	kobject_put(config_kobj);
+	sysfs_remove_empty_dir(kernel_kobj, "config");
 	kmem_cache_destroy(configfs_dir_cachep);
 	configfs_dir_cachep = NULL;
 }
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index c1e7ffb0dab6..5bcb499980d0 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -716,20 +716,17 @@  bool debugfs_initialized(void)
 }
 EXPORT_SYMBOL_GPL(debugfs_initialized);
 
-
-static struct kobject *debug_kobj;
-
 static int __init debugfs_init(void)
 {
 	int retval;
 
-	debug_kobj = kobject_create_and_add("debug", kernel_kobj);
-	if (!debug_kobj)
-		return -EINVAL;
+	retval = sysfs_create_empty_dir(kernel_kobj, "debug");
+	if (retval)
+		return retval;
 
 	retval = register_filesystem(&debug_fs_type);
 	if (retval)
-		kobject_put(debug_kobj);
+		sysfs_remove_empty_dir(kernel_kobj, "debug");
 	else
 		debugfs_registered = true;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 082ac1c97f39..475d9cfa59a9 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1238,7 +1238,6 @@  static void fuse_fs_cleanup(void)
 }
 
 static struct kobject *fuse_kobj;
-static struct kobject *connections_kobj;
 
 static int fuse_sysfs_init(void)
 {
@@ -1250,11 +1249,9 @@  static int fuse_sysfs_init(void)
 		goto out_err;
 	}
 
-	connections_kobj = kobject_create_and_add("connections", fuse_kobj);
-	if (!connections_kobj) {
-		err = -ENOMEM;
+	err = sysfs_create_empty_dir(fuse_kobj, "connections");
+	if (err)
 		goto out_fuse_unregister;
-	}
 
 	return 0;
 
@@ -1266,7 +1263,7 @@  static int fuse_sysfs_init(void)
 
 static void fuse_sysfs_cleanup(void)
 {
-	kobject_put(connections_kobj);
+	sysfs_remove_empty_dir(fuse_kobj, "connections");
 	kobject_put(fuse_kobj);
 }
 
diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index dc43b5f29305..d1caeefd2d1b 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -461,22 +461,18 @@  static struct file_system_type pstore_fs_type = {
 	.kill_sb	= pstore_kill_sb,
 };
 
-static struct kobject *pstore_kobj;
-
 static int __init init_pstore_fs(void)
 {
-	int err = 0;
+	int err;
 
 	/* Create a convenient mount point for people to access pstore */
-	pstore_kobj = kobject_create_and_add("pstore", fs_kobj);
-	if (!pstore_kobj) {
-		err = -ENOMEM;
+	err = sysfs_create_empty_dir(fs_kobj, "pstore");
+	if (err)
 		goto out;
-	}
 
 	err = register_filesystem(&pstore_fs_type);
 	if (err < 0)
-		kobject_put(pstore_kobj);
+		sysfs_remove_empty_dir(fs_kobj, "pstore");
 
 out:
 	return err;
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index d92bdf3b079a..e887c881a4b3 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -631,14 +631,12 @@  bool tracefs_initialized(void)
 	return tracefs_registered;
 }
 
-static struct kobject *trace_kobj;
-
 static int __init tracefs_init(void)
 {
 	int retval;
 
-	trace_kobj = kobject_create_and_add("tracing", kernel_kobj);
-	if (!trace_kobj)
+	retval = sysfs_create_empty_dir(kernel_kobj, "tracing");
+	if (retval)
 		return -EINVAL;
 
 	retval = register_filesystem(&trace_fs_type);
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 469dd547770c..816657b5ef16 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -1924,8 +1924,6 @@  static struct file_system_type cgroup_fs_type = {
 	.kill_sb = cgroup_kill_sb,
 };
 
-static struct kobject *cgroup_kobj;
-
 /**
  * task_cgroup_path - cgroup path of a task in the first cgroup hierarchy
  * @task: target task
@@ -5044,13 +5042,13 @@  int __init cgroup_init(void)
 			ss->bind(init_css_set.subsys[ssid]);
 	}
 
-	cgroup_kobj = kobject_create_and_add("cgroup", fs_kobj);
-	if (!cgroup_kobj)
-		return -ENOMEM;
+	err = sysfs_create_empty_dir(fs_kobj, "cgroup");
+	if (err)
+		return err;
 
 	err = register_filesystem(&cgroup_fs_type);
 	if (err < 0) {
-		kobject_put(cgroup_kobj);
+		sysfs_remove_empty_dir(fs_kobj, "cgroup");
 		return err;
 	}
 
diff --git a/security/inode.c b/security/inode.c
index 91503b79c5f8..d7e5de5ffc59 100644
--- a/security/inode.c
+++ b/security/inode.c
@@ -215,19 +215,17 @@  void securityfs_remove(struct dentry *dentry)
 }
 EXPORT_SYMBOL_GPL(securityfs_remove);
 
-static struct kobject *security_kobj;
-
 static int __init securityfs_init(void)
 {
 	int retval;
 
-	security_kobj = kobject_create_and_add("security", kernel_kobj);
-	if (!security_kobj)
-		return -EINVAL;
+	retval = sysfs_create_empty_dir(kernel_kobj, "security");
+	if (retval)
+		return retval;
 
 	retval = register_filesystem(&fs_type);
 	if (retval)
-		kobject_put(security_kobj);
+		sysfs_remove_empty_dir(kernel_kobj, "security");
 	return retval;
 }
 
diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
index d2787cca1fcb..a3d882729a45 100644
--- a/security/selinux/selinuxfs.c
+++ b/security/selinux/selinuxfs.c
@@ -1853,7 +1853,6 @@  static struct file_system_type sel_fs_type = {
 };
 
 struct vfsmount *selinuxfs_mount;
-static struct kobject *selinuxfs_kobj;
 
 static int __init init_sel_fs(void)
 {
@@ -1862,13 +1861,13 @@  static int __init init_sel_fs(void)
 	if (!selinux_enabled)
 		return 0;
 
-	selinuxfs_kobj = kobject_create_and_add("selinux", fs_kobj);
-	if (!selinuxfs_kobj)
-		return -ENOMEM;
+	err = sysfs_create_empty_dir(fs_kobj, "selinux");
+	if (err)
+		return err;
 
 	err = register_filesystem(&sel_fs_type);
 	if (err) {
-		kobject_put(selinuxfs_kobj);
+		sysfs_remove_empty_dir(fs_kobj, "selinux");
 		return err;
 	}
 
@@ -1887,7 +1886,7 @@  __initcall(init_sel_fs);
 #ifdef CONFIG_SECURITY_SELINUX_DISABLE
 void exit_sel_fs(void)
 {
-	kobject_put(selinuxfs_kobj);
+	sysfs_remove_empty_dir(fs_kobj, "selinux");
 	kern_unmount(selinuxfs_mount);
 	unregister_filesystem(&sel_fs_type);
 }
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index d9682985349e..35079cc8c765 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -2241,16 +2241,16 @@  static const struct file_operations smk_revoke_subj_ops = {
 	.llseek		= generic_file_llseek,
 };
 
-static struct kset *smackfs_kset;
 /**
  * smk_init_sysfs - initialize /sys/fs/smackfs
  *
  */
 static int smk_init_sysfs(void)
 {
-	smackfs_kset = kset_create_and_add("smackfs", NULL, fs_kobj);
-	if (!smackfs_kset)
-		return -ENOMEM;
+	int err;
+	err = sysfs_create_empty_dir(fs_kobj, "smackfs");
+	if (err)
+		return err;
 	return 0;
 }