| Message ID | 0b11ff81143ebfcaa1da26296ee12afcfe41dbb5.1599091832.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: seed fix null ptr, use only main device_list_mutex, and cleanups | expand |
On 3.09.20 г. 3:57 ч., Anand Jain wrote: > The following test case leads to null kobject-being-freed error. > > mount seed /mnt > add sprout to /mnt > umount /mnt > mount sprout to /mnt > delete seed This patch warrants an fstests alongside it! > > kobject: '(null)' (00000000dd2b87e4): is not initialized, yet kobject_put() is being called. > WARNING: CPU: 1 PID: 15784 at lib/kobject.c:736 kobject_put+0x80/0x350 > RIP: 0010:kobject_put+0x80/0x350 > :: > Call Trace: > btrfs_sysfs_remove_devices_dir+0x6e/0x160 [btrfs] > btrfs_rm_device.cold+0xa8/0x298 [btrfs] > btrfs_ioctl+0x206c/0x22a0 [btrfs] > ksys_ioctl+0xe2/0x140 > __x64_sys_ioctl+0x1e/0x29 > do_syscall_64+0x96/0x150 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > RIP: 0033:0x7f4047c6288b > :: > > This is because, at the end of the seed device-delete, we try to remove > the seed's devid sysfs entry. But for the seed devices under the sprout > fs, we don't initialize the devid kobject yet. So this patch initializes > the seed device devid kobject and the device link in the sysfs. This takes > care of the Warning. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > --- > fs/btrfs/sysfs.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index 9853b9acd4bd..98ce955a0879 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -938,9 +938,15 @@ void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs) > void btrfs_sysfs_remove_fs_devices(struct btrfs_fs_devices *fs_devices) > { > struct btrfs_device *device; > + struct btrfs_fs_devices *seed; > > list_for_each_entry(device, &fs_devices->devices, dev_list) > btrfs_sysfs_remove_device(device); > + > + list_for_each_entry(seed, &fs_devices->seed_list, seed_list) { > + list_for_each_entry(device, &seed->devices, dev_list) > + btrfs_sysfs_remove_device(device); > + } > } > > void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info) > @@ -1314,6 +1320,7 @@ int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices) > { > int ret; > struct btrfs_device *device; > + struct btrfs_fs_devices *seed; > > list_for_each_entry(device, &fs_devices->devices, dev_list) { > ret = btrfs_sysfs_add_device(device); > @@ -1321,6 +1328,14 @@ int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices) > return ret; > } > > + list_for_each_entry(seed, &fs_devices->seed_list, seed_list) { > + list_for_each_entry(device, &seed->devices, dev_list) { > + ret = btrfs_sysfs_add_device(device); > + if (ret) > + return ret; > + } > + } > + > return 0; > } > >
On 3/9/20 5:06 pm, Nikolay Borisov wrote: > > > On 3.09.20 г. 3:57 ч., Anand Jain wrote: >> The following test case leads to null kobject-being-freed error. >> >> mount seed /mnt >> add sprout to /mnt >> umount /mnt >> mount sprout to /mnt >> delete seed > > This patch warrants an fstests alongside it! I have been using one which I forgot to send. Now done. Thanks, Anand
On Thu, Sep 03, 2020 at 08:57:42AM +0800, Anand Jain wrote: > The following test case leads to null kobject-being-freed error. > > mount seed /mnt > add sprout to /mnt > umount /mnt > mount sprout to /mnt > delete seed > > kobject: '(null)' (00000000dd2b87e4): is not initialized, yet kobject_put() is being called. > WARNING: CPU: 1 PID: 15784 at lib/kobject.c:736 kobject_put+0x80/0x350 > RIP: 0010:kobject_put+0x80/0x350 > :: > Call Trace: > btrfs_sysfs_remove_devices_dir+0x6e/0x160 [btrfs] > btrfs_rm_device.cold+0xa8/0x298 [btrfs] > btrfs_ioctl+0x206c/0x22a0 [btrfs] > ksys_ioctl+0xe2/0x140 > __x64_sys_ioctl+0x1e/0x29 > do_syscall_64+0x96/0x150 > entry_SYSCALL_64_after_hwframe+0x44/0xa9 > RIP: 0033:0x7f4047c6288b > :: > > This is because, at the end of the seed device-delete, we try to remove > the seed's devid sysfs entry. But for the seed devices under the sprout > fs, we don't initialize the devid kobject yet. So this patch initializes > the seed device devid kobject and the device link in the sysfs. This takes > care of the Warning. So this is the fix but needs 5 more cleanup patches and this makes it too hard to backport. Is there an isolated patch that can be also applied to current 5.9 queue. The test leads to a crash so once the test lands in fstests all testing machines without the fix will crash, so we need fix first, cleanup later. There are also the seed list changes to this need to reflect that somehow.
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 9853b9acd4bd..98ce955a0879 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -938,9 +938,15 @@ void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs) void btrfs_sysfs_remove_fs_devices(struct btrfs_fs_devices *fs_devices) { struct btrfs_device *device; + struct btrfs_fs_devices *seed; list_for_each_entry(device, &fs_devices->devices, dev_list) btrfs_sysfs_remove_device(device); + + list_for_each_entry(seed, &fs_devices->seed_list, seed_list) { + list_for_each_entry(device, &seed->devices, dev_list) + btrfs_sysfs_remove_device(device); + } } void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info) @@ -1314,6 +1320,7 @@ int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices) { int ret; struct btrfs_device *device; + struct btrfs_fs_devices *seed; list_for_each_entry(device, &fs_devices->devices, dev_list) { ret = btrfs_sysfs_add_device(device); @@ -1321,6 +1328,14 @@ int btrfs_sysfs_add_fs_devices(struct btrfs_fs_devices *fs_devices) return ret; } + list_for_each_entry(seed, &fs_devices->seed_list, seed_list) { + list_for_each_entry(device, &seed->devices, dev_list) { + ret = btrfs_sysfs_add_device(device); + if (ret) + return ret; + } + } + return 0; }
The following test case leads to null kobject-being-freed error. mount seed /mnt add sprout to /mnt umount /mnt mount sprout to /mnt delete seed kobject: '(null)' (00000000dd2b87e4): is not initialized, yet kobject_put() is being called. WARNING: CPU: 1 PID: 15784 at lib/kobject.c:736 kobject_put+0x80/0x350 RIP: 0010:kobject_put+0x80/0x350 :: Call Trace: btrfs_sysfs_remove_devices_dir+0x6e/0x160 [btrfs] btrfs_rm_device.cold+0xa8/0x298 [btrfs] btrfs_ioctl+0x206c/0x22a0 [btrfs] ksys_ioctl+0xe2/0x140 __x64_sys_ioctl+0x1e/0x29 do_syscall_64+0x96/0x150 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7f4047c6288b :: This is because, at the end of the seed device-delete, we try to remove the seed's devid sysfs entry. But for the seed devices under the sprout fs, we don't initialize the devid kobject yet. So this patch initializes the seed device devid kobject and the device link in the sysfs. This takes care of the Warning. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/sysfs.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)