On Mon, 7 Jul 2014 12:04:09 +0800, Anand Jain wrote:
>> when one of the device path is missing btrfs_device name is null. So this
>> patch will check for that.
>>
>> stack:
>> BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
>> IP: [<ffffffff812e18c0>] strlen+0x0/0x30
>> [<ffffffffa01cd92a>] ? clone_fs_devices+0xaa/0x160 [btrfs]
>> [<ffffffffa01cdcf7>] btrfs_init_new_device+0x317/0xca0 [btrfs]
>> [<ffffffff81155bca>] ? __kmalloc_track_caller+0x15a/0x1a0
>> [<ffffffffa01d6473>] btrfs_ioctl+0xaa3/0x2860 [btrfs]
>> [<ffffffff81132a6c>] ? handle_mm_fault+0x48c/0x9c0
>> [<ffffffff81192a61>] ? __blkdev_put+0x171/0x180
>> [<ffffffff817a784c>] ? __do_page_fault+0x4ac/0x590
>> [<ffffffff81193426>] ? blkdev_put+0x106/0x110
>> [<ffffffff81179175>] ? mntput+0x35/0x40
>> [<ffffffff8116d4b0>] do_vfs_ioctl+0x460/0x4a0
>> [<ffffffff8115c72e>] ? ____fput+0xe/0x10
>> [<ffffffff81068033>] ? task_work_run+0xb3/0xd0
>> [<ffffffff8116d547>] SyS_ioctl+0x57/0x90
>> [<ffffffff817a793e>] ? do_page_fault+0xe/0x10
>> [<ffffffff817abe52>] system_call_fastpath+0x16/0x1b
>>
>> reproducer:
>> mkfs.btrfs -draid1 -mraid1 /dev/sdg1 /dev/sdg2
>> btrfstune -S 1 /dev/sdg1
>> modprobe -r btrfs && modprobe btrfs
>> mount -o degraded /dev/sdg1 /btrfs
>> btrfs dev add /dev/sdg3 /btrfs
>>
>> Signed-off-by: Anand Jain <[email protected]>
>> Signed-off-by: Miao Xie <[email protected]>
>> ---
>> Changelog v1->v2:
>> - Fix the problem that we forgot to set the missing flag for the cloned
>> device
>> ---
>> fs/btrfs/volumes.c | 25 ++++++++++++++++---------
>> 1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 1891541..4731bd6 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -598,16 +598,23 @@ static struct btrfs_fs_devices
>> *clone_fs_devices(struct btrfs_fs_devices *orig)
>> if (IS_ERR(device))
>> goto error;
>>
>> - /*
>> - * This is ok to do without rcu read locked because we hold the
>> - * uuid mutex so nothing we touch in here is going to disappear.
>> - */
>> - name = rcu_string_strdup(orig_dev->name->str, GFP_NOFS);
>> - if (!name) {
>> - kfree(device);
>> - goto error;
>> + if (orig_dev->missing) {
>> + device->missing = 1;
>> + fs_devices->missing_devices++;
>
> as mentioned in some places we just check name (for missing device)
> and don't set the missing flag so it better to ..
>
> if (orig_dev->missing || !orig_dev->name) {
> device->missing = 1;
> fs_devices->missing_devices++;
I don't think we need check name pointer here because only missing device
doesn't have its own name. Or there is something wrong in the code, so
I add assert in else branch. Am I right?
Thanks
Miao
>
>> + } else {
>> + ASSERT(orig_dev->name);
>> + /*
>> + * This is ok to do without rcu read locked because
>> + * we hold the uuid mutex so nothing we touch in here
>> + * is going to disappear.
>> + */
>> + name = rcu_string_strdup(orig_dev->name->str, GFP_NOFS);
>> + if (!name) {
>> + kfree(device);
>> + goto error;
>> + }
>> + rcu_assign_pointer(device->name, name);
>> }
>> - rcu_assign_pointer(device->name, name);
>>
>> list_add(&device->dev_list, &fs_devices->devices);
>> device->fs_devices = fs_devices;
>>
>
> Thanks, Anand
> .
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
