Future patches will add new syscalls which use these functions. As
this interface won't be used for ioctls only, the EOPNOSUPP is more
appropriate return code.
This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
---
fs/ecryptfs/inode.c | 8 +++++++-
fs/file_attr.c | 12 ++++++++++--
fs/overlayfs/inode.c | 2 +-
3 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
index 493d7f194956..a55c1375127f 100644
--- a/fs/ecryptfs/inode.c
+++ b/fs/ecryptfs/inode.c
@@ -1126,7 +1126,13 @@ static int ecryptfs_removexattr(struct dentry *dentry, struct inode *inode,
static int ecryptfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
{
- return vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
+ int rc;
+
+ rc = vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
+ if (rc == -EOPNOTSUPP)
+ rc = -ENOIOCTLCMD;
+
+ return rc;
}
static int ecryptfs_fileattr_set(struct mnt_idmap *idmap,
diff --git a/fs/file_attr.c b/fs/file_attr.c
index be62d97cc444..4e85fa00c092 100644
--- a/fs/file_attr.c
+++ b/fs/file_attr.c
@@ -79,7 +79,7 @@ int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
int error;
if (!inode->i_op->fileattr_get)
- return -ENOIOCTLCMD;
+ return -EOPNOTSUPP;
error = security_inode_file_getattr(dentry, fa);
if (error)
@@ -229,7 +229,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
int err;
if (!inode->i_op->fileattr_set)
- return -ENOIOCTLCMD;
+ return -EOPNOTSUPP;
if (!inode_owner_or_capable(idmap, inode))
return -EPERM;
@@ -271,6 +271,8 @@ int ioctl_getflags(struct file *file, unsigned int __user *argp)
int err;
err = vfs_fileattr_get(file->f_path.dentry, &fa);
+ if (err == -EOPNOTSUPP)
+ err = -ENOIOCTLCMD;
if (!err)
err = put_user(fa.flags, argp);
return err;
@@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp)
fileattr_fill_flags(&fa, flags);
err = vfs_fileattr_set(idmap, dentry, &fa);
mnt_drop_write_file(file);
+ if (err == -EOPNOTSUPP)
+ err = -ENOIOCTLCMD;
}
}
return err;
@@ -304,6 +308,8 @@ int ioctl_fsgetxattr(struct file *file, void __user *argp)
int err;
err = vfs_fileattr_get(file->f_path.dentry, &fa);
+ if (err == -EOPNOTSUPP)
+ err = -ENOIOCTLCMD;
if (!err)
err = copy_fsxattr_to_user(&fa, argp);
@@ -324,6 +330,8 @@ int ioctl_fssetxattr(struct file *file, void __user *argp)
if (!err) {
err = vfs_fileattr_set(idmap, dentry, &fa);
mnt_drop_write_file(file);
+ if (err == -EOPNOTSUPP)
+ err = -ENOIOCTLCMD;
}
}
return err;
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index 6f0e15f86c21..096d44712bb1 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -721,7 +721,7 @@ int ovl_real_fileattr_get(const struct path *realpath, struct fileattr *fa)
return err;
err = vfs_fileattr_get(realpath->dentry, fa);
- if (err == -ENOIOCTLCMD)
+ if (err == -EOPNOTSUPP)
err = -ENOTTY;
return err;
}
--
2.47.2On 30. 06. 25, 18:20, Andrey Albershteyn wrote: > Future patches will add new syscalls which use these functions. As > this interface won't be used for ioctls only, the EOPNOSUPP is more > appropriate return code. > > This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for > vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate > EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> ... > @@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) > fileattr_fill_flags(&fa, flags); > err = vfs_fileattr_set(idmap, dentry, &fa); > mnt_drop_write_file(file); > + if (err == -EOPNOTSUPP) > + err = -ENOIOCTLCMD; This breaks borg code (unit tests already) as it expects EOPNOTSUPP, not ENOIOCTLCMD/ENOTTY: https://github.com/borgbackup/borg/blob/1c6ef7a200c7f72f8d1204d727fea32168616ceb/src/borg/platform/linux.pyx#L147 I.e. setflags now returns ENOIOCTLCMD/ENOTTY for cases where 6.16 used to return EOPNOTSUPP. This minimal testcase program doing ioctl(fd2, FS_IOC_SETFLAGS, &FS_NODUMP_FL): https://github.com/jirislaby/collected_sources/tree/master/ioctl_setflags dumps in 6.16: sf: ioctl: Operation not supported with the above patch: sf: ioctl: Inappropriate ioctl for device Is this expected? thanks, -- js suse labs
On Mon 06-10-25 13:09:05, Jiri Slaby wrote: > On 30. 06. 25, 18:20, Andrey Albershteyn wrote: > > Future patches will add new syscalls which use these functions. As > > this interface won't be used for ioctls only, the EOPNOSUPP is more > > appropriate return code. > > > > This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for > > vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate > > EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c. > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > ... > > @@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) > > fileattr_fill_flags(&fa, flags); > > err = vfs_fileattr_set(idmap, dentry, &fa); > > mnt_drop_write_file(file); > > + if (err == -EOPNOTSUPP) > > + err = -ENOIOCTLCMD; > > This breaks borg code (unit tests already) as it expects EOPNOTSUPP, not > ENOIOCTLCMD/ENOTTY: > https://github.com/borgbackup/borg/blob/1c6ef7a200c7f72f8d1204d727fea32168616ceb/src/borg/platform/linux.pyx#L147 > > I.e. setflags now returns ENOIOCTLCMD/ENOTTY for cases where 6.16 used to > return EOPNOTSUPP. > > This minimal testcase program doing ioctl(fd2, FS_IOC_SETFLAGS, > &FS_NODUMP_FL): > https://github.com/jirislaby/collected_sources/tree/master/ioctl_setflags > > dumps in 6.16: > sf: ioctl: Operation not supported > > with the above patch: > sf: ioctl: Inappropriate ioctl for device > > Is this expected? No, that's a bug and a clear userspace regression so we need to fix it. I think we need to revert this commit and instead convert ENOIOCTLCMD from vfs_fileattr_get/set() to EOPNOTSUPP in appropriate places. Andrey? Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
On 2025-10-06 17:39:46, Jan Kara wrote: > On Mon 06-10-25 13:09:05, Jiri Slaby wrote: > > On 30. 06. 25, 18:20, Andrey Albershteyn wrote: > > > Future patches will add new syscalls which use these functions. As > > > this interface won't be used for ioctls only, the EOPNOSUPP is more > > > appropriate return code. > > > > > > This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for > > > vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate > > > EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c. > > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > ... > > > @@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) > > > fileattr_fill_flags(&fa, flags); > > > err = vfs_fileattr_set(idmap, dentry, &fa); > > > mnt_drop_write_file(file); > > > + if (err == -EOPNOTSUPP) > > > + err = -ENOIOCTLCMD; > > > > This breaks borg code (unit tests already) as it expects EOPNOTSUPP, not > > ENOIOCTLCMD/ENOTTY: > > https://github.com/borgbackup/borg/blob/1c6ef7a200c7f72f8d1204d727fea32168616ceb/src/borg/platform/linux.pyx#L147 > > > > I.e. setflags now returns ENOIOCTLCMD/ENOTTY for cases where 6.16 used to > > return EOPNOTSUPP. > > > > This minimal testcase program doing ioctl(fd2, FS_IOC_SETFLAGS, > > &FS_NODUMP_FL): > > https://github.com/jirislaby/collected_sources/tree/master/ioctl_setflags > > > > dumps in 6.16: > > sf: ioctl: Operation not supported > > > > with the above patch: > > sf: ioctl: Inappropriate ioctl for device > > > > Is this expected? > > No, that's a bug and a clear userspace regression so we need to fix it. I > think we need to revert this commit and instead convert ENOIOCTLCMD from > vfs_fileattr_get/set() to EOPNOTSUPP in appropriate places. Andrey? I will prepare a patch soon > > Honza > -- > Jan Kara <jack@suse.com> > SUSE Labs, CR > -- - Andrey
On Mon, Oct 06, 2025 at 08:52:32PM +0200, Andrey Albershteyn wrote: > On 2025-10-06 17:39:46, Jan Kara wrote: > > On Mon 06-10-25 13:09:05, Jiri Slaby wrote: > > > On 30. 06. 25, 18:20, Andrey Albershteyn wrote: > > > > Future patches will add new syscalls which use these functions. As > > > > this interface won't be used for ioctls only, the EOPNOSUPP is more > > > > appropriate return code. > > > > > > > > This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for > > > > vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate > > > > EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c. > > > > > > > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> > > > ... > > > > @@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp) > > > > fileattr_fill_flags(&fa, flags); > > > > err = vfs_fileattr_set(idmap, dentry, &fa); > > > > mnt_drop_write_file(file); > > > > + if (err == -EOPNOTSUPP) > > > > + err = -ENOIOCTLCMD; > > > > > > This breaks borg code (unit tests already) as it expects EOPNOTSUPP, not > > > ENOIOCTLCMD/ENOTTY: > > > https://github.com/borgbackup/borg/blob/1c6ef7a200c7f72f8d1204d727fea32168616ceb/src/borg/platform/linux.pyx#L147 > > > > > > I.e. setflags now returns ENOIOCTLCMD/ENOTTY for cases where 6.16 used to > > > return EOPNOTSUPP. > > > > > > This minimal testcase program doing ioctl(fd2, FS_IOC_SETFLAGS, > > > &FS_NODUMP_FL): > > > https://github.com/jirislaby/collected_sources/tree/master/ioctl_setflags > > > > > > dumps in 6.16: > > > sf: ioctl: Operation not supported > > > > > > with the above patch: > > > sf: ioctl: Inappropriate ioctl for device > > > > > > Is this expected? Nope, unintentional regression as Arnd noted. > > > > No, that's a bug and a clear userspace regression so we need to fix it. I > > think we need to revert this commit and instead convert ENOIOCTLCMD from > > vfs_fileattr_get/set() to EOPNOTSUPP in appropriate places. Andrey? > > I will prepare a patch soon Thanks!
On Mon, Oct 6, 2025, at 13:09, Jiri Slaby wrote:
> On 30. 06. 25, 18:20, Andrey Albershteyn wrote:
>> Future patches will add new syscalls which use these functions. As
>> this interface won't be used for ioctls only, the EOPNOSUPP is more
>> appropriate return code.
>>
>> This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
>> vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
>> EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
>>
...
> dumps in 6.16:
> sf: ioctl: Operation not supported
>
> with the above patch:
> sf: ioctl: Inappropriate ioctl for device
>
>
> Is this expected?
This does look like an unintentional bug: As far as I can see, the
-ENOIOCTLCMD was previously used to indicate that a particular filesystem
does not have a fileattr_{get,set} callback at all, while individual
filesystems used EOPNOSUPP to indicate that a particular attribute
flag is unsupported. With the double conversion, both error codes
get turned into a single one.
Arnd
On Mon, Jun 30, 2025 at 06:20:14PM +0200, Andrey Albershteyn wrote:
> Future patches will add new syscalls which use these functions. As
> this interface won't be used for ioctls only, the EOPNOSUPP is more
> appropriate return code.
>
> This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
> vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
> EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
With EOPNOSUPP -> EOPNOTSUPP corrected,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
> ---
> fs/ecryptfs/inode.c | 8 +++++++-
> fs/file_attr.c | 12 ++++++++++--
> fs/overlayfs/inode.c | 2 +-
> 3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 493d7f194956..a55c1375127f 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -1126,7 +1126,13 @@ static int ecryptfs_removexattr(struct dentry *dentry, struct inode *inode,
>
> static int ecryptfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> {
> - return vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
> + int rc;
> +
> + rc = vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
> + if (rc == -EOPNOTSUPP)
> + rc = -ENOIOCTLCMD;
> +
> + return rc;
> }
>
> static int ecryptfs_fileattr_set(struct mnt_idmap *idmap,
> diff --git a/fs/file_attr.c b/fs/file_attr.c
> index be62d97cc444..4e85fa00c092 100644
> --- a/fs/file_attr.c
> +++ b/fs/file_attr.c
> @@ -79,7 +79,7 @@ int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> int error;
>
> if (!inode->i_op->fileattr_get)
> - return -ENOIOCTLCMD;
> + return -EOPNOTSUPP;
>
> error = security_inode_file_getattr(dentry, fa);
> if (error)
> @@ -229,7 +229,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
> int err;
>
> if (!inode->i_op->fileattr_set)
> - return -ENOIOCTLCMD;
> + return -EOPNOTSUPP;
>
> if (!inode_owner_or_capable(idmap, inode))
> return -EPERM;
> @@ -271,6 +271,8 @@ int ioctl_getflags(struct file *file, unsigned int __user *argp)
> int err;
>
> err = vfs_fileattr_get(file->f_path.dentry, &fa);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> if (!err)
> err = put_user(fa.flags, argp);
> return err;
> @@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp)
> fileattr_fill_flags(&fa, flags);
> err = vfs_fileattr_set(idmap, dentry, &fa);
> mnt_drop_write_file(file);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> }
> }
> return err;
> @@ -304,6 +308,8 @@ int ioctl_fsgetxattr(struct file *file, void __user *argp)
> int err;
>
> err = vfs_fileattr_get(file->f_path.dentry, &fa);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> if (!err)
> err = copy_fsxattr_to_user(&fa, argp);
>
> @@ -324,6 +330,8 @@ int ioctl_fssetxattr(struct file *file, void __user *argp)
> if (!err) {
> err = vfs_fileattr_set(idmap, dentry, &fa);
> mnt_drop_write_file(file);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> }
> }
> return err;
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 6f0e15f86c21..096d44712bb1 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -721,7 +721,7 @@ int ovl_real_fileattr_get(const struct path *realpath, struct fileattr *fa)
> return err;
>
> err = vfs_fileattr_get(realpath->dentry, fa);
> - if (err == -ENOIOCTLCMD)
> + if (err == -EOPNOTSUPP)
> err = -ENOTTY;
> return err;
> }
>
> --
> 2.47.2
>
>
On Mon 30-06-25 18:20:14, Andrey Albershteyn wrote: > Future patches will add new syscalls which use these functions. As > this interface won't be used for ioctls only, the EOPNOSUPP is more > appropriate return code. > > This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for > vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate > EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c. > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org> Modulo the small nits already pointed out this looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
>
> Future patches will add new syscalls which use these functions. As
> this interface won't be used for ioctls only, the EOPNOSUPP is more
> appropriate return code.
>
> This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
> vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
> EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> fs/ecryptfs/inode.c | 8 +++++++-
> fs/file_attr.c | 12 ++++++++++--
> fs/overlayfs/inode.c | 2 +-
> 3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 493d7f194956..a55c1375127f 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -1126,7 +1126,13 @@ static int ecryptfs_removexattr(struct dentry *dentry, struct inode *inode,
>
> static int ecryptfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> {
> - return vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
> + int rc;
> +
> + rc = vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
> + if (rc == -EOPNOTSUPP)
> + rc = -ENOIOCTLCMD;
> +
> + return rc;
> }
>
I think the semantics should be
"This patch converts return code of vfs_fileattr_[gs]et and ->fileattr_[gs]et()
from ENOIOCTLCMD to EOPNOSUPP"
ENOIOCTLCMD belongs only in the ioctl frontend, so above conversion
is not needed.
> static int ecryptfs_fileattr_set(struct mnt_idmap *idmap,
> diff --git a/fs/file_attr.c b/fs/file_attr.c
> index be62d97cc444..4e85fa00c092 100644
> --- a/fs/file_attr.c
> +++ b/fs/file_attr.c
> @@ -79,7 +79,7 @@ int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> int error;
>
> if (!inode->i_op->fileattr_get)
> - return -ENOIOCTLCMD;
> + return -EOPNOTSUPP;
>
> error = security_inode_file_getattr(dentry, fa);
> if (error)
> @@ -229,7 +229,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
> int err;
>
> if (!inode->i_op->fileattr_set)
> - return -ENOIOCTLCMD;
> + return -EOPNOTSUPP;
>
> if (!inode_owner_or_capable(idmap, inode))
> return -EPERM;
> @@ -271,6 +271,8 @@ int ioctl_getflags(struct file *file, unsigned int __user *argp)
> int err;
>
> err = vfs_fileattr_get(file->f_path.dentry, &fa);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> if (!err)
> err = put_user(fa.flags, argp);
> return err;
> @@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp)
> fileattr_fill_flags(&fa, flags);
> err = vfs_fileattr_set(idmap, dentry, &fa);
> mnt_drop_write_file(file);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> }
> }
> return err;
> @@ -304,6 +308,8 @@ int ioctl_fsgetxattr(struct file *file, void __user *argp)
> int err;
>
> err = vfs_fileattr_get(file->f_path.dentry, &fa);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> if (!err)
> err = copy_fsxattr_to_user(&fa, argp);
>
> @@ -324,6 +330,8 @@ int ioctl_fssetxattr(struct file *file, void __user *argp)
> if (!err) {
> err = vfs_fileattr_set(idmap, dentry, &fa);
> mnt_drop_write_file(file);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> }
> }
> return err;
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 6f0e15f86c21..096d44712bb1 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -721,7 +721,7 @@ int ovl_real_fileattr_get(const struct path *realpath, struct fileattr *fa)
> return err;
>
> err = vfs_fileattr_get(realpath->dentry, fa);
> - if (err == -ENOIOCTLCMD)
> + if (err == -EOPNOTSUPP)
> err = -ENOTTY;
> return err;
> }
That's the wrong way, because it hides the desired -EOPNOTSUPP
return code from ovl_fileattr_get().
The conversion to -ENOTTY was done for
5b0a414d06c3 ("ovl: fix filattr copy-up failure"),
so please do this instead:
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -722,7 +722,7 @@ int ovl_real_fileattr_get(const struct path
*realpath, struct fileattr *fa)
err = vfs_fileattr_get(realpath->dentry, fa);
if (err == -ENOIOCTLCMD)
- err = -ENOTTY;
+ err = -EOPNOTSUPP;
return err;
}
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -178,7 +178,7 @@ static int ovl_copy_fileattr(struct inode *inode,
const struct path *old,
err = ovl_real_fileattr_get(old, &oldfa);
if (err) {
/* Ntfs-3g returns -EINVAL for "no fileattr support" */
- if (err == -ENOTTY || err == -EINVAL)
+ if (err == -ENOTTY || err == -EINVAL || err == -EOPNOTSUPP)
return 0;
pr_warn("failed to retrieve lower fileattr (%pd2, err=%i)\n",
old->dentry, err);
Thanks,
Amir.
On Tue 01-07-25 08:05:45, Amir Goldstein wrote:
> On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> >
> > Future patches will add new syscalls which use these functions. As
> > this interface won't be used for ioctls only, the EOPNOSUPP is more
> > appropriate return code.
> >
> > This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
> > vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
> > EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
> >
> > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
...
> > --- a/fs/overlayfs/inode.c
> > +++ b/fs/overlayfs/inode.c
> > @@ -721,7 +721,7 @@ int ovl_real_fileattr_get(const struct path *realpath, struct fileattr *fa)
> > return err;
> >
> > err = vfs_fileattr_get(realpath->dentry, fa);
> > - if (err == -ENOIOCTLCMD)
> > + if (err == -EOPNOTSUPP)
> > err = -ENOTTY;
> > return err;
> > }
>
> That's the wrong way, because it hides the desired -EOPNOTSUPP
> return code from ovl_fileattr_get().
>
> The conversion to -ENOTTY was done for
> 5b0a414d06c3 ("ovl: fix filattr copy-up failure"),
> so please do this instead:
>
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -722,7 +722,7 @@ int ovl_real_fileattr_get(const struct path
> *realpath, struct fileattr *fa)
>
> err = vfs_fileattr_get(realpath->dentry, fa);
> if (err == -ENOIOCTLCMD)
> - err = -ENOTTY;
> + err = -EOPNOTSUPP;
Is this really needed? AFAICS nobody returns ENOIOCTLCMD after this
patch...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
On Tue, Jul 1, 2025 at 2:51 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 01-07-25 08:05:45, Amir Goldstein wrote:
> > On Mon, Jun 30, 2025 at 6:20 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> > >
> > > Future patches will add new syscalls which use these functions. As
> > > this interface won't be used for ioctls only, the EOPNOSUPP is more
> > > appropriate return code.
> > >
> > > This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
> > > vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
> > > EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
> > >
> > > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ...
> > > --- a/fs/overlayfs/inode.c
> > > +++ b/fs/overlayfs/inode.c
> > > @@ -721,7 +721,7 @@ int ovl_real_fileattr_get(const struct path *realpath, struct fileattr *fa)
> > > return err;
> > >
> > > err = vfs_fileattr_get(realpath->dentry, fa);
> > > - if (err == -ENOIOCTLCMD)
> > > + if (err == -EOPNOTSUPP)
> > > err = -ENOTTY;
> > > return err;
> > > }
> >
> > That's the wrong way, because it hides the desired -EOPNOTSUPP
> > return code from ovl_fileattr_get().
> >
> > The conversion to -ENOTTY was done for
> > 5b0a414d06c3 ("ovl: fix filattr copy-up failure"),
> > so please do this instead:
> >
> > --- a/fs/overlayfs/inode.c
> > +++ b/fs/overlayfs/inode.c
> > @@ -722,7 +722,7 @@ int ovl_real_fileattr_get(const struct path
> > *realpath, struct fileattr *fa)
> >
> > err = vfs_fileattr_get(realpath->dentry, fa);
> > if (err == -ENOIOCTLCMD)
> > - err = -ENOTTY;
> > + err = -EOPNOTSUPP;
>
> Is this really needed? AFAICS nobody returns ENOIOCTLCMD after this
> patch...
you are right it is not needed
Attaching the patch with missing bits of fuse and overlayfs to make this
conversion complete.
Christian, please squash my patch
and afterward make sure there is no conversion remaining in
ovl_real_fileattr_get() as well as in ecryptfs_fileattr_get()
Both those helpers should return the value they
got from vfs_fileattr_get() as is.
Thanks,
Amir.
nit: typo in commit subject and description: Missing T in EOPNO*T*SUPP.
But please do not resend whole patch series just because of this.
That is not needed.
On Monday 30 June 2025 18:20:14 Andrey Albershteyn wrote:
> Future patches will add new syscalls which use these functions. As
> this interface won't be used for ioctls only, the EOPNOSUPP is more
> appropriate return code.
>
> This patch converts return code from ENOIOCTLCMD to EOPNOSUPP for
> vfs_fileattr_get and vfs_fileattr_set. To save old behavior translate
> EOPNOSUPP back for current users - overlayfs, encryptfs and fs/ioctl.c.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> fs/ecryptfs/inode.c | 8 +++++++-
> fs/file_attr.c | 12 ++++++++++--
> fs/overlayfs/inode.c | 2 +-
> 3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
> index 493d7f194956..a55c1375127f 100644
> --- a/fs/ecryptfs/inode.c
> +++ b/fs/ecryptfs/inode.c
> @@ -1126,7 +1126,13 @@ static int ecryptfs_removexattr(struct dentry *dentry, struct inode *inode,
>
> static int ecryptfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> {
> - return vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
> + int rc;
> +
> + rc = vfs_fileattr_get(ecryptfs_dentry_to_lower(dentry), fa);
> + if (rc == -EOPNOTSUPP)
> + rc = -ENOIOCTLCMD;
> +
> + return rc;
> }
>
> static int ecryptfs_fileattr_set(struct mnt_idmap *idmap,
> diff --git a/fs/file_attr.c b/fs/file_attr.c
> index be62d97cc444..4e85fa00c092 100644
> --- a/fs/file_attr.c
> +++ b/fs/file_attr.c
> @@ -79,7 +79,7 @@ int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa)
> int error;
>
> if (!inode->i_op->fileattr_get)
> - return -ENOIOCTLCMD;
> + return -EOPNOTSUPP;
>
> error = security_inode_file_getattr(dentry, fa);
> if (error)
> @@ -229,7 +229,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry,
> int err;
>
> if (!inode->i_op->fileattr_set)
> - return -ENOIOCTLCMD;
> + return -EOPNOTSUPP;
>
> if (!inode_owner_or_capable(idmap, inode))
> return -EPERM;
> @@ -271,6 +271,8 @@ int ioctl_getflags(struct file *file, unsigned int __user *argp)
> int err;
>
> err = vfs_fileattr_get(file->f_path.dentry, &fa);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> if (!err)
> err = put_user(fa.flags, argp);
> return err;
> @@ -292,6 +294,8 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp)
> fileattr_fill_flags(&fa, flags);
> err = vfs_fileattr_set(idmap, dentry, &fa);
> mnt_drop_write_file(file);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> }
> }
> return err;
> @@ -304,6 +308,8 @@ int ioctl_fsgetxattr(struct file *file, void __user *argp)
> int err;
>
> err = vfs_fileattr_get(file->f_path.dentry, &fa);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> if (!err)
> err = copy_fsxattr_to_user(&fa, argp);
>
> @@ -324,6 +330,8 @@ int ioctl_fssetxattr(struct file *file, void __user *argp)
> if (!err) {
> err = vfs_fileattr_set(idmap, dentry, &fa);
> mnt_drop_write_file(file);
> + if (err == -EOPNOTSUPP)
> + err = -ENOIOCTLCMD;
> }
> }
> return err;
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 6f0e15f86c21..096d44712bb1 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -721,7 +721,7 @@ int ovl_real_fileattr_get(const struct path *realpath, struct fileattr *fa)
> return err;
>
> err = vfs_fileattr_get(realpath->dentry, fa);
> - if (err == -ENOIOCTLCMD)
> + if (err == -EOPNOTSUPP)
> err = -ENOTTY;
> return err;
> }
>
> --
> 2.47.2
>
© 2016 - 2025 Red Hat, Inc.