[PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Xiao Yang]

Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
directories.

Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
---
 fs/overlayfs/file.c      | 5 ++---
 fs/overlayfs/overlayfs.h | 2 ++
 fs/overlayfs/readdir.c   | 2 ++
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
index 0d940e29d62b..94ad7f9c9c76 100644
--- a/fs/overlayfs/file.c
+++ b/fs/overlayfs/file.c
@@ -640,7 +640,7 @@ static long ovl_ioctl_set_fsxflags(struct file *file, unsigned int cmd,
 				   ovl_fsxflags_to_iflags(fa.fsx_xflags));
 }
 
-static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	long ret;
 
@@ -665,8 +665,7 @@ static long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	return ret;
 }
 
-static long ovl_compat_ioctl(struct file *file, unsigned int cmd,
-			     unsigned long arg)
+long ovl_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	switch (cmd) {
 	case FS_IOC32_GETFLAGS:
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index 29bc1ec699e7..bfb499314dcc 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -301,6 +301,8 @@ bool ovl_is_metacopy_dentry(struct dentry *dentry);
 char *ovl_get_redirect_xattr(struct dentry *dentry, int padding);
 ssize_t ovl_getxattr(struct dentry *dentry, char *name, char **value,
 		     size_t padding);
+long ovl_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
+long ovl_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 
 static inline bool ovl_is_impuredir(struct dentry *dentry)
 {
diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 6918b98faeb6..cde909b6fe7d 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -945,6 +945,8 @@ const struct file_operations ovl_dir_operations = {
 	.llseek		= ovl_dir_llseek,
 	.fsync		= ovl_dir_fsync,
 	.release	= ovl_dir_release,
+	.unlocked_ioctl = ovl_ioctl,
+	.compat_ioctl   = ovl_compat_ioctl,
 };
 
 int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
-- 
2.25.1

Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Amir Goldstein]

On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
> directories.
>
> Signed-off-by: Xiao Yang <yangx.jy@cn.fujitsu.com>
> ---

This change is buggy. I had already posted it and self NACKed myself [1].

You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.

As long as you are changing ovl_ioctl(), please also take the second
commit on that
branch to replace the open coded capability check with the
vfs_ioc_setflags_prepare()
generic helper.

Thanks,
Amir.


[1] https://lore.kernel.org/linux-unionfs/CAOQ4uxhRgL2sMok7xsAZN6cZXSfoPxx=O8ADE=72+Ta3hGoLbw@mail.gmail.com/
[2] https://github.com/amir73il/linux/commits/ovl-shutdown

Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Xiao Yang]

On 2020/9/21 16:17, Amir Goldstein wrote:
> On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang<yangx.jy@cn.fujitsu.com>  wrote:
>> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
>> directories.
>>
>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>> ---
> This change is buggy. I had already posted it and self NACKed myself [1].
>
> You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.
>
> As long as you are changing ovl_ioctl(), please also take the second
> commit on that
> branch to replace the open coded capability check with the
> vfs_ioc_setflags_prepare()
> generic helper.
Hi Amir,

Thanks a lot for your quick reply. :-)
I will try to read and understand the metioned patches on your 
ovl-shutdown branch.

Best Regards,
Xiao Yang
> Thanks,
> Amir.
>
>
> [1] https://lore.kernel.org/linux-unionfs/CAOQ4uxhRgL2sMok7xsAZN6cZXSfoPxx=O8ADE=72+Ta3hGoLbw@mail.gmail.com/
> [2] https://github.com/amir73il/linux/commits/ovl-shutdown
>
>
> .
>

Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Amir Goldstein]

On Mon, Sep 21, 2020 at 11:55 AM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> On 2020/9/21 16:17, Amir Goldstein wrote:
> > On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang<yangx.jy@cn.fujitsu.com>  wrote:
> >> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
> >> directories.
> >>
> >> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> >> ---
> > This change is buggy. I had already posted it and self NACKed myself [1].
> >
> > You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.
> >
> > As long as you are changing ovl_ioctl(), please also take the second
> > commit on that
> > branch to replace the open coded capability check with the
> > vfs_ioc_setflags_prepare()
> > generic helper.
> Hi Amir,
>
> Thanks a lot for your quick reply. :-)
> I will try to read and understand the metioned patches on your
> ovl-shutdown branch.

Please also verify my claim in the patch commit message, that the
the test result of xfstest generic/079 changes from "notrun" to "success".

Thanks,
Amir.

Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Xiao Yang]

On 2020/9/21 17:09, Amir Goldstein wrote:
> On Mon, Sep 21, 2020 at 11:55 AM Xiao Yang<yangx.jy@cn.fujitsu.com>  wrote:
>> On 2020/9/21 16:17, Amir Goldstein wrote:
>>> On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang<yangx.jy@cn.fujitsu.com>   wrote:
>>>> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
>>>> directories.
>>>>
>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>> ---
>>> This change is buggy. I had already posted it and self NACKed myself [1].
>>>
>>> You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.
>>>
>>> As long as you are changing ovl_ioctl(), please also take the second
>>> commit on that
>>> branch to replace the open coded capability check with the
>>> vfs_ioc_setflags_prepare()
>>> generic helper.
>> Hi Amir,
>>
>> Thanks a lot for your quick reply. :-)
>> I will try to read and understand the metioned patches on your
>> ovl-shutdown branch.
> Please also verify my claim in the patch commit message, that the
> the test result of xfstest generic/079 changes from "notrun" to "success".
Hi Amir,

With your patches, I have confirmed that generic/079 actually changed from
"notrun" to "success".  Besides, one minor issue:
Could we avoid the following compiler warning?
-------------------------------------------------
fs/overlayfs/readdir.c: In function ‘ovl_dir_real_file’:
fs/overlayfs/readdir.c:883:37: warning: passing argument 1 of 
‘ovl_dir_open_realfile’ discards ‘const’ qualifier from pointer target 
type [-Wdiscarded-qualifiers]
   883 |    realfile = ovl_dir_open_realfile(file, &upperpath);
       |                                     ^~~~
fs/overlayfs/readdir.c:842:56: note: expected ‘struct file *’ but 
argument is of type ‘const struct file *’
   842 | static struct file *ovl_dir_open_realfile(struct file *file,
       |                                           ~~~~~~~~~~~~~^~~~
-------------------------------------------------

Best Regards,
Xiao Yang
> Thanks,
> Amir.
>
>
> .
>

Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Amir Goldstein]

On Tue, Sep 22, 2020 at 11:15 AM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> On 2020/9/21 17:09, Amir Goldstein wrote:
> > On Mon, Sep 21, 2020 at 11:55 AM Xiao Yang<yangx.jy@cn.fujitsu.com>  wrote:
> >> On 2020/9/21 16:17, Amir Goldstein wrote:
> >>> On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang<yangx.jy@cn.fujitsu.com>   wrote:
> >>>> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
> >>>> directories.
> >>>>
> >>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> >>>> ---
> >>> This change is buggy. I had already posted it and self NACKed myself [1].
> >>>
> >>> You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.
> >>>
> >>> As long as you are changing ovl_ioctl(), please also take the second
> >>> commit on that
> >>> branch to replace the open coded capability check with the
> >>> vfs_ioc_setflags_prepare()
> >>> generic helper.
> >> Hi Amir,
> >>
> >> Thanks a lot for your quick reply. :-)
> >> I will try to read and understand the metioned patches on your
> >> ovl-shutdown branch.
> > Please also verify my claim in the patch commit message, that the
> > the test result of xfstest generic/079 changes from "notrun" to "success".
> Hi Amir,
>
> With your patches, I have confirmed that generic/079 actually changed from
> "notrun" to "success".  Besides, one minor issue:
> Could we avoid the following compiler warning?
> -------------------------------------------------
> fs/overlayfs/readdir.c: In function ‘ovl_dir_real_file’:
> fs/overlayfs/readdir.c:883:37: warning: passing argument 1 of
> ‘ovl_dir_open_realfile’ discards ‘const’ qualifier from pointer target
> type [-Wdiscarded-qualifiers]
>    883 |    realfile = ovl_dir_open_realfile(file, &upperpath);
>        |                                     ^~~~
> fs/overlayfs/readdir.c:842:56: note: expected ‘struct file *’ but
> argument is of type ‘const struct file *’
>    842 | static struct file *ovl_dir_open_realfile(struct file *file,
>        |                                           ~~~~~~~~~~~~~^~~~
> -------------------------------------------------
>

Shouldn't be a problem to change ovl_dir_open_realfile()
to take a const struct file * argument I think.

Thanks,
Amir.

Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Xiao Yang]

On 2020/9/22 20:01, Amir Goldstein wrote:
> On Tue, Sep 22, 2020 at 11:15 AM Xiao Yang<yangx.jy@cn.fujitsu.com>  wrote:
>> On 2020/9/21 17:09, Amir Goldstein wrote:
>>> On Mon, Sep 21, 2020 at 11:55 AM Xiao Yang<yangx.jy@cn.fujitsu.com>   wrote:
>>>> On 2020/9/21 16:17, Amir Goldstein wrote:
>>>>> On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang<yangx.jy@cn.fujitsu.com>    wrote:
>>>>>> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
>>>>>> directories.
>>>>>>
>>>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
>>>>>> ---
>>>>> This change is buggy. I had already posted it and self NACKed myself [1].
>>>>>
>>>>> You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.
>>>>>
>>>>> As long as you are changing ovl_ioctl(), please also take the second
>>>>> commit on that
>>>>> branch to replace the open coded capability check with the
>>>>> vfs_ioc_setflags_prepare()
>>>>> generic helper.
>>>> Hi Amir,
>>>>
>>>> Thanks a lot for your quick reply. :-)
>>>> I will try to read and understand the metioned patches on your
>>>> ovl-shutdown branch.
>>> Please also verify my claim in the patch commit message, that the
>>> the test result of xfstest generic/079 changes from "notrun" to "success".
>> Hi Amir,
>>
>> With your patches, I have confirmed that generic/079 actually changed from
>> "notrun" to "success".  Besides, one minor issue:
>> Could we avoid the following compiler warning?
>> -------------------------------------------------
>> fs/overlayfs/readdir.c: In function ‘ovl_dir_real_file’:
>> fs/overlayfs/readdir.c:883:37: warning: passing argument 1 of
>> ‘ovl_dir_open_realfile’ discards ‘const’ qualifier from pointer target
>> type [-Wdiscarded-qualifiers]
>>     883 |    realfile = ovl_dir_open_realfile(file,&upperpath);
>>         |                                     ^~~~
>> fs/overlayfs/readdir.c:842:56: note: expected ‘struct file *’ but
>> argument is of type ‘const struct file *’
>>     842 | static struct file *ovl_dir_open_realfile(struct file *file,
>>         |                                           ~~~~~~~~~~~~~^~~~
>> -------------------------------------------------
>>
> Shouldn't be a problem to change ovl_dir_open_realfile()
> to take a const struct file * argument I think.
Hi Amir,

Other than the compiler warning I tested your patches on our
enviroment and didn't find any issue, so add:
Reviewed-by: Xiao Yang <yangx.jy@.cn.fujisu.com>

Thank you for sharing these patches again. :-)

Best Regards,
Xiao Yang
> Thanks,
> Amir.
>
>
> .
>

Re: [PATCH] ovl: Support FS_IOC_[SG]ETFLAGS and FS_IOC_FS[SG]ETXATTR ioctls on directories

[Amir Goldstein]

On Fri, Sep 25, 2020 at 6:13 PM Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> On 2020/9/22 20:01, Amir Goldstein wrote:
> > On Tue, Sep 22, 2020 at 11:15 AM Xiao Yang<yangx.jy@cn.fujitsu.com>  wrote:
> >> On 2020/9/21 17:09, Amir Goldstein wrote:
> >>> On Mon, Sep 21, 2020 at 11:55 AM Xiao Yang<yangx.jy@cn.fujitsu.com>   wrote:
> >>>> On 2020/9/21 16:17, Amir Goldstein wrote:
> >>>>> On Mon, Sep 21, 2020 at 10:41 AM Xiao Yang<yangx.jy@cn.fujitsu.com>    wrote:
> >>>>>> Factor out ovl_ioctl() and ovl_compat_ioctl() and take use of them for
> >>>>>> directories.
> >>>>>>
> >>>>>> Signed-off-by: Xiao Yang<yangx.jy@cn.fujitsu.com>
> >>>>>> ---
> >>>>> This change is buggy. I had already posted it and self NACKed myself [1].
> >>>>>
> >>>>> You can find an hopefully non-buggy version of it on my ovl-shutdown [2] branch.
> >>>>>
> >>>>> As long as you are changing ovl_ioctl(), please also take the second
> >>>>> commit on that
> >>>>> branch to replace the open coded capability check with the
> >>>>> vfs_ioc_setflags_prepare()
> >>>>> generic helper.
> >>>> Hi Amir,
> >>>>
> >>>> Thanks a lot for your quick reply. :-)
> >>>> I will try to read and understand the metioned patches on your
> >>>> ovl-shutdown branch.
> >>> Please also verify my claim in the patch commit message, that the
> >>> the test result of xfstest generic/079 changes from "notrun" to "success".
> >> Hi Amir,
> >>
> >> With your patches, I have confirmed that generic/079 actually changed from
> >> "notrun" to "success".  Besides, one minor issue:
> >> Could we avoid the following compiler warning?
> >> -------------------------------------------------
> >> fs/overlayfs/readdir.c: In function ‘ovl_dir_real_file’:
> >> fs/overlayfs/readdir.c:883:37: warning: passing argument 1 of
> >> ‘ovl_dir_open_realfile’ discards ‘const’ qualifier from pointer target
> >> type [-Wdiscarded-qualifiers]
> >>     883 |    realfile = ovl_dir_open_realfile(file,&upperpath);
> >>         |                                     ^~~~
> >> fs/overlayfs/readdir.c:842:56: note: expected ‘struct file *’ but
> >> argument is of type ‘const struct file *’
> >>     842 | static struct file *ovl_dir_open_realfile(struct file *file,
> >>         |                                           ~~~~~~~~~~~~~^~~~
> >> -------------------------------------------------
> >>
> > Shouldn't be a problem to change ovl_dir_open_realfile()
> > to take a const struct file * argument I think.
> Hi Amir,
>
> Other than the compiler warning I tested your patches on our
> enviroment and didn't find any issue, so add:
> Reviewed-by: Xiao Yang <yangx.jy@.cn.fujisu.com>
>
> Thank you for sharing these patches again. :-)

Please post the fixed patches rebased on top of
git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git overlayfs-next.

Please leave my signed-off-by and add your own as well.

Thanks,
Amir.