Re: [PATCH 2/4] ksmbd: remove filename in ksmbd_file

[Namjae Jeon <linkinjeon@kernel.org>]

2022-03-01 13:14 GMT+09:00, Sergey Senozhatsky <senozhatsky@chromium.org>:
> On (22/03/01 08:48), Namjae Jeon wrote:
>> -char *convert_to_nt_pathname(char *filename)
>> +char *convert_to_nt_pathname(struct ksmbd_share_config *share,
>> +			     struct path *path)
>>  {
>> -	char *ab_pathname;
>> +	char *pathname, *ab_pathname, *nt_pathname = NULL;
>> +	int share_path_len = strlen(share->path);
>>
>> -	if (strlen(filename) == 0)
>> -		filename = "\\";
>> +	pathname = kmalloc(PATH_MAX, GFP_KERNEL);
>> +	if (!pathname)
>> +		return ERR_PTR(-EACCES);
>>
>> -	ab_pathname = kstrdup(filename, GFP_KERNEL);
>> -	if (!ab_pathname)
>> -		return NULL;
>> +	ab_pathname = d_path(path, pathname, PATH_MAX);
>> +	if (IS_ERR(ab_pathname)) {
>> +		nt_pathname = ERR_PTR(-EACCES);
>> +		goto free_pathname;
>> +	}
>> +
>> +	if (strncmp(ab_pathname, share->path, share_path_len)) {
>> +		nt_pathname = ERR_PTR(-EACCES);
>> +		goto free_pathname;
>> +	}
>> +
>> +	nt_pathname = kzalloc(strlen(&ab_pathname[share_path_len]) + 1,
>> GFP_KERNEL);
>> +	if (!nt_pathname) {
>> +		nt_pathname = ERR_PTR(-ENOMEM);
>> +		goto free_pathname;
>> +	}
>> +	if (ab_pathname[share_path_len] == '\0')
>> +		strcpy(nt_pathname, "/");
>> +	strcat(nt_pathname, &ab_pathname[share_path_len]);
>> +
>> +	ksmbd_conv_path_to_windows(nt_pathname);
>>
>> -	ksmbd_conv_path_to_windows(ab_pathname);
>> -	return ab_pathname;
>> +free_pathname:
>> +	kfree(pathname);
>> +	return nt_pathname;
>>  }
>
> convert_to_nt_pathname() can return NULL
I can not find where this function return NULL.. Initializing NULL for
nt_pathname is unnecessary.

>
>> +	filename = convert_to_nt_pathname(work->tcon->share_conf,
>> &fp->filp->f_path);
>> +	if (IS_ERR(filename))
>> +		return PTR_ERR(filename);
>
> I don't think this will catch NULL nt_pathname return.
>