[Bug 27912] New: Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[Bug 27912] New: Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=27912

           Summary: Set 'err' in ext4_init_fs() if kset_create_and_add()
                    fails
           Product: File System
           Version: 2.5
    Kernel Version: 2.6.38-rc2
          Platform: All
        OS/Version: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: ext4
        AssignedTo: fs_ext4@kernel-bugs.osdl.org
        ReportedBy: dame_eugene@mail.ru
        Regression: No


Set 'err' in ext4_init_fs() if kset_create_and_add() fails
------------------------

In ext4_init_fs() (super.c), consider the following fragment:

ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj);
if (!ext4_kset)
    goto out4;

If kset_create_and_add() fails for some reason, 'err' variable will remain 0,
so ext4_init_fs() will return 0 despite this failure. It will look like as if
the module has initialized itself successfully while it has not. 

On my system this results in a kernel oops as soon as I try to unload ext4
module. To be exact, when destroy_inodecache() is invoked, it tries to call
kmem_cache_destroy(ext4_inode_cachep) but ext4_inode_cachep is NULL because it
was not initialized due to that incomplete init. So, 

BUG_ON(!cachep || in_interrupt()) 

triggers in kmem_cache_destroy().

The problem can be reproduced as follows (it is assumed that ext4 is compiled
as a module): load ext4.ko, unload it and then load it again. For some reason,
/sys/fs/ext4/ directory is not deleted when the module is unloaded and that is
why kset_create_and_add() fails when I try to load it the second time. From the
system log:

---------------------------------
[ 3077.802232] WARNING: at
/home/eugene/distrib/kernel/mainline.git/linux-2.6/fs/sysfs/dir.c:455
sysfs_add_one+0x8a/0xb0()
[ 3077.802236] Hardware name: VirtualBox
[ 3077.802239] sysfs: cannot create duplicate filename '/fs/ext4'
[ 3077.802242] Modules linked in: ext4(+) jbd2 crc16 kedr_controller
kedr_fsim_indicator_kmalloc kedr_fsim_indicator_common
kedr_fsim_user_space_access kedr_fsim_cmm kedr_fault_simulation kedr_trace
kedr_base fuse snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device edd af_packet
mperf loop dm_mod snd_intel8x0 snd_ac97_codec ac97_bus e1000 snd_pcm snd_timer
ppdev button sg snd soundcore snd_page_alloc parport_pc sr_mod parport cdrom
i2c_piix4 ac pcspkr ohci_hcd ehci_hcd usbcore rtc_cmos sd_mod rtc_core rtc_lib
fan processor ata_generic thermal thermal_sys hwmon ata_piix ahci libahci
libata scsi_mod [last unloaded: kedr_base]
[ 3077.802275] Pid: 5388, comm: modprobe Not tainted 2.6.38-rc2-testbox+ #1
[ 3077.802278] Call Trace:
[ 3077.802286]  [<c02450fd>] ? warn_slowpath_common+0x6d/0xa0
[ 3077.802290]  [<c035267a>] ? sysfs_add_one+0x8a/0xb0
[ 3077.802293]  [<c035267a>] ? sysfs_add_one+0x8a/0xb0
[ 3077.802298]  [<c02451ae>] ? warn_slowpath_fmt+0x2e/0x30
[ 3077.802301]  [<c035267a>] ? sysfs_add_one+0x8a/0xb0
[ 3077.802305]  [<c03526fc>] ? create_dir+0x5c/0xa0
[ 3077.802309]  [<c03527de>] ? sysfs_create_dir+0x6e/0xa0
[ 3077.802314]  [<c03d337d>] ? kobject_add_internal+0x9d/0x230
[ 3077.802318]  [<c03dd383>] ? kvasprintf+0x43/0x60
[ 3077.802322]  [<c03d3a87>] ? kset_register+0x27/0x60
[ 3077.802326]  [<c03d3b27>] ? kset_create_and_add+0x67/0xa0
[ 3077.802336]  [<f8fc4143>] ? ext4_init_fs+0x37/0x114 [ext4]
[ 3077.802340]  [<c0201230>] ? do_one_initcall+0x30/0x170
[ 3077.802349]  [<f8fc410c>] ? ext4_init_fs+0x0/0x114 [ext4]
[ 3077.802354]  [<c0278936>] ? sys_init_module+0x116/0x19c0
[ 3077.802359]  [<c0202ddc>] ? sysenter_do_call+0x12/0x28
[ 3077.802362] ---[ end trace 8b7776f89b6ab60b ]---
[ 3077.802367] kobject_add_internal failed for ext4 with -EEXIST, don't try to
register things with the same name in the same directory.
[ 3077.802371] Pid: 5388, comm: modprobe Tainted: G        W  
2.6.38-rc2-testbox+ #1
[ 3077.802374] Call Trace:
[ 3077.802378]  [<c03d3434>] ? kobject_add_internal+0x154/0x230
[ 3077.802382]  [<c03dd383>] ? kvasprintf+0x43/0x60
[ 3077.802386]  [<c03d3a87>] ? kset_register+0x27/0x60
[ 3077.802389]  [<c03d3b27>] ? kset_create_and_add+0x67/0xa0
[ 3077.802398]  [<f8fc4143>] ? ext4_init_fs+0x37/0x114 [ext4]
[ 3077.802402]  [<c0201230>] ? do_one_initcall+0x30/0x170
[ 3077.802411]  [<f8fc410c>] ? ext4_init_fs+0x0/0x114 [ext4]
[ 3077.802416]  [<c0278936>] ? sys_init_module+0x116/0x19c0
[ 3077.802420]  [<c0202ddc>] ? sysenter_do_call+0x12/0x28 
---------------------------------

I suppose a trivial change in ext4_init_fs() would be enough to fix this
problem:

ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj);
if (!ext4_kset) {
    err = -EEXIST;
    goto out4;
}

Not sure about which error code to return though. As far as I can see from the
code, kset_create_and_add() can fail due to other reasons as well (out of
memory conditions, for example).

Besides that, it is still unclear to me why that sysfs directory is not deleted
when ext4.ko is unloaded. kset_unregister(ext4_kset) seems to be in place in
ext4_exit_fs(). I could probably analyze this a bit further. Anyway, it is
another story. Should I file another bug for this?

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[Bug 27912] Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=27912


Tao Ma <tm@tao.ma> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tm@tao.ma




--- Comment #1 from Tao Ma <tm@tao.ma>  2011-02-01 03:04:48 ---
Hi Eugene,
This is already fixed by Eric.
http://marc.info/?l=linux-ext4&m=129527644524410&w=2

If you can test it, that would be great.

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[Bug 27912] Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=27912





--- Comment #2 from Eugene A. Shatokhin <dame_eugene@mail.ru>  2011-02-01 10:27:36 ---
Ah, that is why there were 3 references to ext4_kset right before
kset_unregister() rather than just one! 

Yes I will try the patch, thank you. 

However, it does not seem to fix the original problem I reported: 'err' remains
0 in ext4_init_fs() if kset_create_and_add() fails for some reason. I suppose
it is quite easy to fix though.

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[Bug 27912] Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=27912





--- Comment #3 from Eugene A. Shatokhin <dame_eugene@mail.ru>  2011-02-01 21:07:13 ---
I have just built and tested the patched ext4.ko module. Yes, the problem with
/sys/fs/ext4 is now fixed as it was expected. 

Obviously, the patch does not fix that problem with 'err' that I reported. So
the patch probably needs to be changed a little.

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[Bug 27912] Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=27912


Alan <alan@lxorguk.ukuu.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |alan@lxorguk.ukuu.org.uk
         Resolution|                            |CODE_FIX




-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

Re: [Bug 27912] Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[Theodore Ts'o]

On Tue, Aug 14, 2012 at 03:55:52PM +0000, bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=27912

Actually, this wasn't quite fixed.  Here's a fix which I'll include in
the ext4 tree....

						- Ted

>From b0f1e9fa10363b60334ba7837080da91de425be0 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Tue, 14 Aug 2012 23:02:17 -0400
Subject: [PATCH] ext4: return an error if kset_create_and_add fails in
 ext4_init_fs()

In the very unlikely case that kset_create_and_add() fails when the
ext4.ko module is being loaded (or during kernel startup) set err so
that it's clear that the module load failed.

https://bugzilla.kernel.org/show_bug.cgi?id=27912

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e7ccbe5..603023b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5281,8 +5281,10 @@ static int __init ext4_init_fs(void)
 	if (err)
 		goto out6;
 	ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj);
-	if (!ext4_kset)
+	if (!ext4_kset) {
+		err = -ENOMEM;
 		goto out5;
+	}
 	ext4_proc_root = proc_mkdir("fs/ext4", NULL);
 
 	err = ext4_init_feat_adverts();
-- 
1.7.12.rc0.22.gcdd159b

[Bug 27912] Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=27912





--- Comment #4 from Theodore Tso <tytso@mit.edu>  2012-08-15 03:03:37 ---
On Tue, Aug 14, 2012 at 03:55:52PM +0000, bugzilla-daemon@bugzilla.kernel.org
wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=27912

Actually, this wasn't quite fixed.  Here's a fix which I'll include in
the ext4 tree....

                        - Ted

>From b0f1e9fa10363b60334ba7837080da91de425be0 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <tytso@mit.edu>
Date: Tue, 14 Aug 2012 23:02:17 -0400
Subject: [PATCH] ext4: return an error if kset_create_and_add fails in
 ext4_init_fs()

In the very unlikely case that kset_create_and_add() fails when the
ext4.ko module is being loaded (or during kernel startup) set err so
that it's clear that the module load failed.

https://bugzilla.kernel.org/show_bug.cgi?id=27912

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
---
 fs/ext4/super.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e7ccbe5..603023b 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5281,8 +5281,10 @@ static int __init ext4_init_fs(void)
     if (err)
         goto out6;
     ext4_kset = kset_create_and_add("ext4", NULL, fs_kobj);
-    if (!ext4_kset)
+    if (!ext4_kset) {
+        err = -ENOMEM;
         goto out5;
+    }
     ext4_proc_root = proc_mkdir("fs/ext4", NULL);

     err = ext4_init_feat_adverts();

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.

[Bug 27912] Set 'err' in ext4_init_fs() if kset_create_and_add() fails

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=27912


Florian Mickler <florian@mickler.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |florian@mickler.org




--- Comment #5 from Florian Mickler <florian@mickler.org>  2012-10-15 21:24:05 ---
A patch referencing this bug report has been merged in Linux v3.7-rc1:

commit 0e376b1e3ccedee49cb8cc6b652fbc1e7c15eeef
Author: Theodore Ts'o <tytso@mit.edu>
Date:   Fri Aug 17 10:04:17 2012 -0400

    ext4: return an error if kset_create_and_add fails in ext4_init_fs()

-- 
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching the assignee of the bug.