| Message ID | 20250329143312.1350603-3-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | etfsxattrat() followup patches | expand |
On Sat, Mar 29, 2025 at 3:33 PM Amir Goldstein <amir73il@gmail.com> wrote: > > With getfsxattrat() syscall, filesystem may use this field to report > its supported xflags. Zero mask value means that supported flags are > not advertized. > > With setfsxattrat() syscall, userspace may use this field to declare > which xflags and fields are being set. Zero mask value means that > all known xflags and fields are being set. > > Programs that call getfsxattrat() to fill struct fsxattr before calling > setfsxattrat() will not be affected by this change, but it allows > programs that call setfsxattrat() without calling getfsxattrat() to make > changes to some xflags and fields without knowing or changing the values > of unrelated xflags and fields. > > Link: https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@kernel.org/ > Cc: Pali Rohár <pali@kernel.org> > Cc: Andrey Albershteyn <aalbersh@redhat.com> > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > fs/ioctl.c | 35 +++++++++++++++++++++++++++++------ > include/linux/fileattr.h | 1 + > include/uapi/linux/fs.h | 3 ++- > 3 files changed, 32 insertions(+), 7 deletions(-) > > diff --git a/fs/ioctl.c b/fs/ioctl.c > index b19858db4c432..a4838b3e7de90 100644 > --- a/fs/ioctl.c > +++ b/fs/ioctl.c > @@ -540,10 +540,13 @@ EXPORT_SYMBOL(vfs_fileattr_get); > > void fileattr_to_fsxattr(const struct fileattr *fa, struct fsxattr *fsx) > { > - __u32 mask = FS_XFALGS_MASK; > + /* Filesystem may or may not advertize supported xflags */ > + __u32 fs_mask = fa->fsx_xflags_mask & FS_XFALGS_MASK; > + __u32 mask = fs_mask ?: FS_XFALGS_MASK; > > memset(fsx, 0, sizeof(struct fsxattr)); > fsx->fsx_xflags = fa->fsx_xflags & mask; > + fsx->fsx_xflags_mask = fs_mask; > fsx->fsx_extsize = fa->fsx_extsize; > fsx->fsx_nextents = fa->fsx_nextents; > fsx->fsx_projid = fa->fsx_projid; > @@ -562,6 +565,8 @@ int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa) > struct fsxattr xfa; > > fileattr_to_fsxattr(fa, &xfa); > + /* FS_IOC_FSGETXATTR ioctl does not report supported fsx_xflags_mask */ > + xfa.fsx_xflags_mask = 0; > > if (copy_to_user(ufa, &xfa, sizeof(xfa))) > return -EFAULT; > @@ -572,16 +577,30 @@ EXPORT_SYMBOL(copy_fsxattr_to_user); > > int fsxattr_to_fileattr(const struct fsxattr *fsx, struct fileattr *fa) > { > - __u32 mask = FS_XFALGS_MASK; > + /* User may or may not provide custom xflags mask */ > + __u32 mask = fsx->fsx_xflags_mask ?: FS_XFALGS_MASK; > > - if (fsx->fsx_xflags & ~mask) > + if ((fsx->fsx_xflags & ~mask) || (mask & ~FS_XFALGS_MASK)) > return -EINVAL; > > fileattr_fill_xflags(fa, fsx->fsx_xflags); > fa->fsx_xflags &= ~FS_XFLAG_RDONLY_MASK; > - fa->fsx_extsize = fsx->fsx_extsize; > - fa->fsx_projid = fsx->fsx_projid; > - fa->fsx_cowextsize = fsx->fsx_cowextsize; > + fa->fsx_xflags_mask = fsx->fsx_xflags_mask; > + /* > + * If flags mask is specified, we copy the fields value only if the > + * relevant flag is set in the mask. > + */ > + if (!mask || (mask & (FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT))) > + fa->fsx_extsize = fsx->fsx_extsize; > + if (!mask || (mask & FS_XFLAG_COWEXTSIZE)) > + fa->fsx_cowextsize = fsx->fsx_cowextsize; > + /* > + * To save a mask flag (i.e. FS_XFLAG_PROJID), require setting values > + * of fsx_projid and FS_XFLAG_PROJINHERIT flag values together. > + * For a non-directory, FS_XFLAG_PROJINHERIT flag value should be 0. > + */ > + if (!mask || (mask & FS_XFLAG_PROJINHERIT)) > + fa->fsx_projid = fsx->fsx_projid; Sorry, I ended up initializing the mask without a user provided mask to FS_XFALGS_MASK, so these (!mask ||) conditions are not needed. Thanks, Amir.
On Saturday 29 March 2025 15:43:06 Amir Goldstein wrote: > On Sat, Mar 29, 2025 at 3:33 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > With getfsxattrat() syscall, filesystem may use this field to report > > its supported xflags. Zero mask value means that supported flags are > > not advertized. > > > > With setfsxattrat() syscall, userspace may use this field to declare > > which xflags and fields are being set. Zero mask value means that > > all known xflags and fields are being set. > > > > Programs that call getfsxattrat() to fill struct fsxattr before calling > > setfsxattrat() will not be affected by this change, but it allows > > programs that call setfsxattrat() without calling getfsxattrat() to make > > changes to some xflags and fields without knowing or changing the values > > of unrelated xflags and fields. > > > > Link: https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@kernel.org/ > > Cc: Pali Rohár <pali@kernel.org> > > Cc: Andrey Albershteyn <aalbersh@redhat.com> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > fs/ioctl.c | 35 +++++++++++++++++++++++++++++------ > > include/linux/fileattr.h | 1 + > > include/uapi/linux/fs.h | 3 ++- > > 3 files changed, 32 insertions(+), 7 deletions(-) > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c > > index b19858db4c432..a4838b3e7de90 100644 > > --- a/fs/ioctl.c > > +++ b/fs/ioctl.c > > @@ -540,10 +540,13 @@ EXPORT_SYMBOL(vfs_fileattr_get); > > > > void fileattr_to_fsxattr(const struct fileattr *fa, struct fsxattr *fsx) > > { > > - __u32 mask = FS_XFALGS_MASK; > > + /* Filesystem may or may not advertize supported xflags */ > > + __u32 fs_mask = fa->fsx_xflags_mask & FS_XFALGS_MASK; > > + __u32 mask = fs_mask ?: FS_XFALGS_MASK; > > > > memset(fsx, 0, sizeof(struct fsxattr)); > > fsx->fsx_xflags = fa->fsx_xflags & mask; > > + fsx->fsx_xflags_mask = fs_mask; > > fsx->fsx_extsize = fa->fsx_extsize; > > fsx->fsx_nextents = fa->fsx_nextents; > > fsx->fsx_projid = fa->fsx_projid; > > @@ -562,6 +565,8 @@ int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa) > > struct fsxattr xfa; > > > > fileattr_to_fsxattr(fa, &xfa); > > + /* FS_IOC_FSGETXATTR ioctl does not report supported fsx_xflags_mask */ > > + xfa.fsx_xflags_mask = 0; > > > > if (copy_to_user(ufa, &xfa, sizeof(xfa))) > > return -EFAULT; > > @@ -572,16 +577,30 @@ EXPORT_SYMBOL(copy_fsxattr_to_user); > > > > int fsxattr_to_fileattr(const struct fsxattr *fsx, struct fileattr *fa) > > { > > - __u32 mask = FS_XFALGS_MASK; > > + /* User may or may not provide custom xflags mask */ > > + __u32 mask = fsx->fsx_xflags_mask ?: FS_XFALGS_MASK; > > > > - if (fsx->fsx_xflags & ~mask) > > + if ((fsx->fsx_xflags & ~mask) || (mask & ~FS_XFALGS_MASK)) > > return -EINVAL; > > > > fileattr_fill_xflags(fa, fsx->fsx_xflags); > > fa->fsx_xflags &= ~FS_XFLAG_RDONLY_MASK; > > - fa->fsx_extsize = fsx->fsx_extsize; > > - fa->fsx_projid = fsx->fsx_projid; > > - fa->fsx_cowextsize = fsx->fsx_cowextsize; > > + fa->fsx_xflags_mask = fsx->fsx_xflags_mask; > > + /* > > + * If flags mask is specified, we copy the fields value only if the > > + * relevant flag is set in the mask. > > + */ > > + if (!mask || (mask & (FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT))) > > + fa->fsx_extsize = fsx->fsx_extsize; > > + if (!mask || (mask & FS_XFLAG_COWEXTSIZE)) > > + fa->fsx_cowextsize = fsx->fsx_cowextsize; > > + /* > > + * To save a mask flag (i.e. FS_XFLAG_PROJID), require setting values > > + * of fsx_projid and FS_XFLAG_PROJINHERIT flag values together. > > + * For a non-directory, FS_XFLAG_PROJINHERIT flag value should be 0. > > + */ > > + if (!mask || (mask & FS_XFLAG_PROJINHERIT)) > > + fa->fsx_projid = fsx->fsx_projid; > > Sorry, I ended up initializing the mask without a user provided mask > to FS_XFALGS_MASK, so these (!mask ||) conditions are not needed. > > Thanks, > Amir. And there is a typo: FS_XFLAGS_MASK
On Sat, Mar 29, 2025 at 3:44 PM Pali Rohár <pali@kernel.org> wrote: > > On Saturday 29 March 2025 15:43:06 Amir Goldstein wrote: > > On Sat, Mar 29, 2025 at 3:33 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > With getfsxattrat() syscall, filesystem may use this field to report > > > its supported xflags. Zero mask value means that supported flags are > > > not advertized. > > > > > > With setfsxattrat() syscall, userspace may use this field to declare > > > which xflags and fields are being set. Zero mask value means that > > > all known xflags and fields are being set. > > > > > > Programs that call getfsxattrat() to fill struct fsxattr before calling > > > setfsxattrat() will not be affected by this change, but it allows > > > programs that call setfsxattrat() without calling getfsxattrat() to make > > > changes to some xflags and fields without knowing or changing the values > > > of unrelated xflags and fields. > > > > > > Link: https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@kernel.org/ > > > Cc: Pali Rohár <pali@kernel.org> > > > Cc: Andrey Albershteyn <aalbersh@redhat.com> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > --- > > > fs/ioctl.c | 35 +++++++++++++++++++++++++++++------ > > > include/linux/fileattr.h | 1 + > > > include/uapi/linux/fs.h | 3 ++- > > > 3 files changed, 32 insertions(+), 7 deletions(-) > > > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c > > > index b19858db4c432..a4838b3e7de90 100644 > > > --- a/fs/ioctl.c > > > +++ b/fs/ioctl.c > > > @@ -540,10 +540,13 @@ EXPORT_SYMBOL(vfs_fileattr_get); > > > > > > void fileattr_to_fsxattr(const struct fileattr *fa, struct fsxattr *fsx) > > > { > > > - __u32 mask = FS_XFALGS_MASK; > > > + /* Filesystem may or may not advertize supported xflags */ > > > + __u32 fs_mask = fa->fsx_xflags_mask & FS_XFALGS_MASK; > > > + __u32 mask = fs_mask ?: FS_XFALGS_MASK; > > > > > > memset(fsx, 0, sizeof(struct fsxattr)); > > > fsx->fsx_xflags = fa->fsx_xflags & mask; > > > + fsx->fsx_xflags_mask = fs_mask; > > > fsx->fsx_extsize = fa->fsx_extsize; > > > fsx->fsx_nextents = fa->fsx_nextents; > > > fsx->fsx_projid = fa->fsx_projid; > > > @@ -562,6 +565,8 @@ int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa) > > > struct fsxattr xfa; > > > > > > fileattr_to_fsxattr(fa, &xfa); > > > + /* FS_IOC_FSGETXATTR ioctl does not report supported fsx_xflags_mask */ > > > + xfa.fsx_xflags_mask = 0; > > > > > > if (copy_to_user(ufa, &xfa, sizeof(xfa))) > > > return -EFAULT; > > > @@ -572,16 +577,30 @@ EXPORT_SYMBOL(copy_fsxattr_to_user); > > > > > > int fsxattr_to_fileattr(const struct fsxattr *fsx, struct fileattr *fa) > > > { > > > - __u32 mask = FS_XFALGS_MASK; > > > + /* User may or may not provide custom xflags mask */ > > > + __u32 mask = fsx->fsx_xflags_mask ?: FS_XFALGS_MASK; > > > > > > - if (fsx->fsx_xflags & ~mask) > > > + if ((fsx->fsx_xflags & ~mask) || (mask & ~FS_XFALGS_MASK)) > > > return -EINVAL; > > > > > > fileattr_fill_xflags(fa, fsx->fsx_xflags); > > > fa->fsx_xflags &= ~FS_XFLAG_RDONLY_MASK; > > > - fa->fsx_extsize = fsx->fsx_extsize; > > > - fa->fsx_projid = fsx->fsx_projid; > > > - fa->fsx_cowextsize = fsx->fsx_cowextsize; > > > + fa->fsx_xflags_mask = fsx->fsx_xflags_mask; > > > + /* > > > + * If flags mask is specified, we copy the fields value only if the > > > + * relevant flag is set in the mask. > > > + */ > > > + if (!mask || (mask & (FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT))) > > > + fa->fsx_extsize = fsx->fsx_extsize; > > > + if (!mask || (mask & FS_XFLAG_COWEXTSIZE)) > > > + fa->fsx_cowextsize = fsx->fsx_cowextsize; > > > + /* > > > + * To save a mask flag (i.e. FS_XFLAG_PROJID), require setting values > > > + * of fsx_projid and FS_XFLAG_PROJINHERIT flag values together. > > > + * For a non-directory, FS_XFLAG_PROJINHERIT flag value should be 0. > > > + */ > > > + if (!mask || (mask & FS_XFLAG_PROJINHERIT)) > > > + fa->fsx_projid = fsx->fsx_projid; > > > > Sorry, I ended up initializing the mask without a user provided mask > > to FS_XFALGS_MASK, so these (!mask ||) conditions are not needed. > > > > Thanks, > > Amir. > > And there is a typo: FS_XFLAGS_MASK Oops. Fixed typo and braino and pushed to https://github.com/amir73il/linux/commits/fsxattr Thanks, Amir.
diff --git a/fs/ioctl.c b/fs/ioctl.c index b19858db4c432..a4838b3e7de90 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -540,10 +540,13 @@ EXPORT_SYMBOL(vfs_fileattr_get); void fileattr_to_fsxattr(const struct fileattr *fa, struct fsxattr *fsx) { - __u32 mask = FS_XFALGS_MASK; + /* Filesystem may or may not advertize supported xflags */ + __u32 fs_mask = fa->fsx_xflags_mask & FS_XFALGS_MASK; + __u32 mask = fs_mask ?: FS_XFALGS_MASK; memset(fsx, 0, sizeof(struct fsxattr)); fsx->fsx_xflags = fa->fsx_xflags & mask; + fsx->fsx_xflags_mask = fs_mask; fsx->fsx_extsize = fa->fsx_extsize; fsx->fsx_nextents = fa->fsx_nextents; fsx->fsx_projid = fa->fsx_projid; @@ -562,6 +565,8 @@ int copy_fsxattr_to_user(const struct fileattr *fa, struct fsxattr __user *ufa) struct fsxattr xfa; fileattr_to_fsxattr(fa, &xfa); + /* FS_IOC_FSGETXATTR ioctl does not report supported fsx_xflags_mask */ + xfa.fsx_xflags_mask = 0; if (copy_to_user(ufa, &xfa, sizeof(xfa))) return -EFAULT; @@ -572,16 +577,30 @@ EXPORT_SYMBOL(copy_fsxattr_to_user); int fsxattr_to_fileattr(const struct fsxattr *fsx, struct fileattr *fa) { - __u32 mask = FS_XFALGS_MASK; + /* User may or may not provide custom xflags mask */ + __u32 mask = fsx->fsx_xflags_mask ?: FS_XFALGS_MASK; - if (fsx->fsx_xflags & ~mask) + if ((fsx->fsx_xflags & ~mask) || (mask & ~FS_XFALGS_MASK)) return -EINVAL; fileattr_fill_xflags(fa, fsx->fsx_xflags); fa->fsx_xflags &= ~FS_XFLAG_RDONLY_MASK; - fa->fsx_extsize = fsx->fsx_extsize; - fa->fsx_projid = fsx->fsx_projid; - fa->fsx_cowextsize = fsx->fsx_cowextsize; + fa->fsx_xflags_mask = fsx->fsx_xflags_mask; + /* + * If flags mask is specified, we copy the fields value only if the + * relevant flag is set in the mask. + */ + if (!mask || (mask & (FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT))) + fa->fsx_extsize = fsx->fsx_extsize; + if (!mask || (mask & FS_XFLAG_COWEXTSIZE)) + fa->fsx_cowextsize = fsx->fsx_cowextsize; + /* + * To save a mask flag (i.e. FS_XFLAG_PROJID), require setting values + * of fsx_projid and FS_XFLAG_PROJINHERIT flag values together. + * For a non-directory, FS_XFLAG_PROJINHERIT flag value should be 0. + */ + if (!mask || (mask & FS_XFLAG_PROJINHERIT)) + fa->fsx_projid = fsx->fsx_projid; return 0; } @@ -594,6 +613,10 @@ static int copy_fsxattr_from_user(struct fileattr *fa, if (copy_from_user(&xfa, ufa, sizeof(xfa))) return -EFAULT; + /* FS_IOC_FSSETXATTR ioctl does not support user fsx_xflags_mask */ + if (xfa.fsx_xflags_mask) + return -EINVAL; + return fsxattr_to_fileattr(&xfa, fa); } diff --git a/include/linux/fileattr.h b/include/linux/fileattr.h index f682bfc7749dd..93102423b7c95 100644 --- a/include/linux/fileattr.h +++ b/include/linux/fileattr.h @@ -44,6 +44,7 @@ struct fileattr { u32 flags; /* flags (FS_IOC_GETFLAGS/FS_IOC_SETFLAGS) */ /* struct fsxattr: */ u32 fsx_xflags; /* xflags field value (get/set) */ + u32 fsx_xflags_mask;/* xflags valid mask (get/set) */ u32 fsx_extsize; /* extsize field value (get/set)*/ u32 fsx_nextents; /* nextents field value (get) */ u32 fsx_projid; /* project identifier (get/set) */ diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h index 0ae21596e25a5..5c35c415ca8fa 100644 --- a/include/uapi/linux/fs.h +++ b/include/uapi/linux/fs.h @@ -145,7 +145,8 @@ struct fsxattr { __u32 fsx_nextents; /* nextents field value (get) */ __u32 fsx_projid; /* project identifier (get/set) */ __u32 fsx_cowextsize; /* CoW extsize field value (get/set)*/ - unsigned char fsx_pad[8]; + __u32 fsx_xflags_mask;/* xflags valid mask (get/set) */ + unsigned char fsx_pad[4]; }; #define FSXATTR_SIZE_VER0 28
With getfsxattrat() syscall, filesystem may use this field to report its supported xflags. Zero mask value means that supported flags are not advertized. With setfsxattrat() syscall, userspace may use this field to declare which xflags and fields are being set. Zero mask value means that all known xflags and fields are being set. Programs that call getfsxattrat() to fill struct fsxattr before calling setfsxattrat() will not be affected by this change, but it allows programs that call setfsxattrat() without calling getfsxattrat() to make changes to some xflags and fields without knowing or changing the values of unrelated xflags and fields. Link: https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@kernel.org/ Cc: Pali Rohár <pali@kernel.org> Cc: Andrey Albershteyn <aalbersh@redhat.com> Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- fs/ioctl.c | 35 +++++++++++++++++++++++++++++------ include/linux/fileattr.h | 1 + include/uapi/linux/fs.h | 3 ++- 3 files changed, 32 insertions(+), 7 deletions(-)