Re: [PATCH] hostfs: fix opening file with overlayfs

Re: [PATCH] hostfs: fix opening file with overlayfs

[Richard Weinberger]

CC'ing VFS folks

Am 01.05.2016 um 11:02 schrieb Toshikuni Fukaya:
> If a file on overlayfs using hostfs as a lowerdir is opened, it would
> be failed by ENOENT because hostfs resolves a host path by a host root
> path stored on the hostfs superblock referred by file->f_path.dentry,
> but it is replaced with overlayfs's one starting from the
> commit 4bacc9c9234c ("overlayfs: Make f_path always point to the
> overlay and f_inode to the underlay").
> 
> We replace using file->f_path.dentry directly with calling
> "file_dentry" from a commit d101a125954e ("fs: add file_dentry()") to
> fix the issue.

Is there a reason why not all instances of file->f_path.dentry have
been replaced by file_dentry()?
I smell more fallout...

Thanks,
//richard

> Signed-off-by: Toshikuni Fukaya <toshiq2@gmail.com>
> ---
>  fs/hostfs/hostfs_kern.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
> index 7016653..26a9f43 100644
> --- a/fs/hostfs/hostfs_kern.c
> +++ b/fs/hostfs/hostfs_kern.c
> @@ -284,7 +284,7 @@ static int hostfs_readdir(struct file *file, struct dir_context *ctx)
>  	int error, len;
>  	unsigned int type;
>  
> -	name = dentry_name(file->f_path.dentry);
> +	name = dentry_name(file_dentry(file));
>  	if (name == NULL)
>  		return -ENOMEM;
>  	dir = open_dir(name, &error);
> @@ -323,7 +323,7 @@ retry:
>  	if (mode & FMODE_WRITE)
>  		r = w = 1;
>  
> -	name = dentry_name(file->f_path.dentry);
> +	name = dentry_name(file_dentry(file));
>  	if (name == NULL)
>  		return -ENOMEM;
>  
> 

Re: [PATCH] hostfs: fix opening file with overlayfs

[Miklos Szeredi]

On Sun, May 1, 2016 at 10:21 PM, Richard Weinberger <richard@nod.at> wrote:
> CC'ing VFS folks
>
> Am 01.05.2016 um 11:02 schrieb Toshikuni Fukaya:
>> If a file on overlayfs using hostfs as a lowerdir is opened, it would
>> be failed by ENOENT because hostfs resolves a host path by a host root
>> path stored on the hostfs superblock referred by file->f_path.dentry,
>> but it is replaced with overlayfs's one starting from the
>> commit 4bacc9c9234c ("overlayfs: Make f_path always point to the
>> overlay and f_inode to the underlay").
>>
>> We replace using file->f_path.dentry directly with calling
>> "file_dentry" from a commit d101a125954e ("fs: add file_dentry()") to
>> fix the issue.
>
> Is there a reason why not all instances of file->f_path.dentry have
> been replaced by file_dentry()?
> I smell more fallout...

It's perfectly safe to do.  But automatic conversion is not advised,
as there are cases where using f_path.dentry is just hiding bugs.
Each case should be thought about separately, and preferably converted
to file_inode() if possible.

The hostfs patch looks good, though.

Acked-by: Miklos Szeredi <mszeredi@redhat.com>

Thanks,
Miklos





>
> Thanks,
> //richard
>
>> Signed-off-by: Toshikuni Fukaya <toshiq2@gmail.com>
>> ---
>>  fs/hostfs/hostfs_kern.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c
>> index 7016653..26a9f43 100644
>> --- a/fs/hostfs/hostfs_kern.c
>> +++ b/fs/hostfs/hostfs_kern.c
>> @@ -284,7 +284,7 @@ static int hostfs_readdir(struct file *file, struct dir_context *ctx)
>>       int error, len;
>>       unsigned int type;
>>
>> -     name = dentry_name(file->f_path.dentry);
>> +     name = dentry_name(file_dentry(file));
>>       if (name == NULL)
>>               return -ENOMEM;
>>       dir = open_dir(name, &error);
>> @@ -323,7 +323,7 @@ retry:
>>       if (mode & FMODE_WRITE)
>>               r = w = 1;
>>
>> -     name = dentry_name(file->f_path.dentry);
>> +     name = dentry_name(file_dentry(file));
>>       if (name == NULL)
>>               return -ENOMEM;
>>
>>