Re: [PATCH 2/2] smb: client: prevent out-of-bounds dfs_info3_param access in copy_ref_data()

[Paulo Alcantara]

Paulo Alcantara <pc@xxxxxxxxxxxxx> writes:

> Fredric Cover <fredric.cover.lkernel@xxxxxxxxx> writes:
>
>> Currently, copy_ref_data() assumes that numrefs > 0, and unconditinally
>> accesses refs[0]. If copy_ref_data() somehow is passed an empty list,
>> this would cause major out-of-bounds write and read problems.
>>
>> Enforce numrefs > 0.
>>
>> Signed-off-by: Fredric Cover <fredric.cover.lkernel@xxxxxxxxx>
>> ---
>>  fs/smb/client/dfs_cache.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c
>> index b0388c460499..b51c8355d73d 100644
>> --- a/fs/smb/client/dfs_cache.c
>> +++ b/fs/smb/client/dfs_cache.c
>> @@ -389,6 +389,9 @@ static int copy_ref_data(const struct dfs_info3_param *refs, int numrefs,
>>  	struct cache_dfs_tgt *target;
>>  	int i;
>>  
>> +	if (WARN_ON_ONCE(numrefs <= 0))
>> +		return -EINVAL;
>
> I don't understand why this check is necessary.  All target referrals
> come from get_dfs_referral(), and in case @numrefs < 0,
                                            ^ @numrefs <= 0                                        

Sorry for the typo.