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

[Paulo Alcantara <pc@manguebit.org>]

Fredric Cover <fredric.cover.lkernel@gmail.com> 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@gmail.com>
> ---
>  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,
parse_dfs_referrals() would have returned -ENOENT and copy_ref_data()
wouldn't be called.