Re: [PATCH] btrfs: Check if tgt_device is not null

From: Nikolay Borisov <nborisov@suse.com>
To: dsterba@suse.cz, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: Check if tgt_device is not null
Date: Wed, 12 Jul 2017 20:19:36 +0300	[thread overview]
Message-ID: <b9f6453f-5bd4-80bc-ffcb-27a40d6b4fe8@suse.com> (raw)
In-Reply-To: <20170712150319.GF2866@twin.jikos.cz>

On 12.07.2017 18:03, David Sterba wrote:
> On Wed, Jul 12, 2017 at 11:39:22AM +0300, Nikolay Borisov wrote:
>> btrfs_err_in_rcu indiscriminately dereferences tgt_device to access its
>> ->name member in an error path. However, couple of lines below there is code
>> which checks whether tgt_device is not NULL. Let's be consistent and check if
>> the tgt_device is NULL before dereferencing it.
> 
> The question is if tgt_device can be really NULL. From what I see I
> don't think so. The target device is the one we're writing to, so we've
> used it through the entire dev-replace. Source device can be null if
> we're replacing a missing device.

So in this case the if tgt_null check can be removed a couple of lines
below.

> 
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  fs/btrfs/dev-replace.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
>> index bee3edeea7a3..e2a16cb8f7f3 100644
>> --- a/fs/btrfs/dev-replace.c
>> +++ b/fs/btrfs/dev-replace.c
>> @@ -541,7 +541,8 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
>>  				 src_device->missing ? "<missing disk>" :
>>  				 rcu_str_deref(src_device->name),
>>  				 src_device->devid,
>> -				 rcu_str_deref(tgt_device->name), scrub_ret);
>> +				 tgt_device ? rcu_str_deref(tgt_device->name) :
>> +				 "<missing disk>", scrub_ret);
>>  		btrfs_dev_replace_unlock(dev_replace, 1);
>>  		mutex_unlock(&fs_info->chunk_mutex);
>>  		mutex_unlock(&fs_info->fs_devices->device_list_mutex);