Bug 27912
| Summary: | Set 'err' in ext4_init_fs() if kset_create_and_add() fails | ||
|---|---|---|---|
| Product: | File System | Reporter: | Eugene A. Shatokhin (eugene.shatokhin) |
| Component: | ext4 | Assignee: | fs_ext4 (fs_ext4) |
| Status: | RESOLVED CODE_FIX | ||
| Severity: | normal | CC: | alan, florian, tm |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.38-rc2 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
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. 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. 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. 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(); 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() |
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?