Re: [PATCH v2 2/4] fs: add FS_IOC_FSSETXATTRAT and FS_IOC_FSGETXATTRAT

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[fix fsdevel address]

On Mon, May 20, 2024 at 10:03 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Mon, May 20, 2024 at 7:46 PM Andrey Albershteyn <aalbersh@xxxxxxxxxx> wrote:
> >
> > XFS has project quotas which could be attached to a directory. All
> > new inodes in these directories inherit project ID set on parent
> > directory.
> >
> > The project is created from userspace by opening and calling
> > FS_IOC_FSSETXATTR on each inode. This is not possible for special
> > files such as FIFO, SOCK, BLK etc. as opening them returns a special
> > inode from VFS. Therefore, some inodes are left with empty project
> > ID. Those inodes then are not shown in the quota accounting but
> > still exist in the directory.
> >
> > This patch adds two new ioctls which allows userspace, such as
> > xfs_quota, to set project ID on special files by using parent
> > directory to open FS inode. This will let xfs_quota set ID on all
> > inodes and also reset it when project is removed. Also, as
> > vfs_fileattr_set() is now will called on special files too, let's
> > forbid any other attributes except projid and nextents (symlink can
> > have one).
> >
> > Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> > ---
> >  fs/ioctl.c              | 93 +++++++++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/fs.h | 11 +++++
> >  2 files changed, 104 insertions(+)
> >
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index 1d5abfdf0f22..3e3aacb6ea6e 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -22,6 +22,7 @@
> >  #include <linux/mount.h>
> >  #include <linux/fscrypt.h>
> >  #include <linux/fileattr.h>
> > +#include <linux/namei.h>
> >
> >  #include "internal.h"
> >
> > @@ -647,6 +648,19 @@ static int fileattr_set_prepare(struct inode *inode,
> >         if (fa->fsx_cowextsize == 0)
> >                 fa->fsx_xflags &= ~FS_XFLAG_COWEXTSIZE;
> >
> > +       /*
> > +        * The only use case for special files is to set project ID, forbid any
> > +        * other attributes
> > +        */
> > +       if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))) {
> > +               if (fa->fsx_xflags & ~FS_XFLAG_PROJINHERIT)
> > +                       return -EINVAL;
> > +               if (!S_ISLNK(inode->i_mode) && fa->fsx_nextents)
> > +                       return -EINVAL;
> > +               if (fa->fsx_extsize || fa->fsx_cowextsize)
> > +                       return -EINVAL;
> > +       }
> > +
> >         return 0;
> >  }
> >
> > @@ -763,6 +777,79 @@ static int ioctl_fssetxattr(struct file *file, void __user *argp)
> >         return err;
> >  }
> >
> > +static int ioctl_fsgetxattrat(struct file *file, void __user *argp)
> > +{
> > +       struct path filepath;
> > +       struct fsxattrat fsxat;
> > +       struct fileattr fa;
> > +       int error;
> > +
> > +       if (!S_ISDIR(file_inode(file)->i_mode))
> > +               return -EBADF;
>
> So the *only* thing that is done with the fd of the ioctl is to verify
> that it is a directory fd - there is no verification that this fd is on the
> same sb as the path to act on.
>
> Was this the intention? It does not make a lot of sense to me
> and AFAIK there is no precedent to an API like this.
>
> There are ioctls that operate on the filesystem using any
> fd on that fs, such as FS_IOC_GETFS{UUID,SYSFSPATH}
> and maybe the closest example to what you are trying to add
> XFS_IOC_BULKSTAT.
>
> Trying to think of a saner API for this - perhaps pass an O_PATH
> fd without any filename in struct fsxattrat, saving you also the
> headache of passing a variable length string in an ioctl.
>
> Then atfile = fdget_raw(fsxat.atfd) and verify that atfile->f_path
> and file->f_path are on the same sb before proceeding to operate
> on atfile->f_path.dentry.
>
> The (maybe more sane) alternative is to add syscalls instead of
> ioctls, but I won't force you to go there...
>
> What do everyone think?
>
> Thanks,
> Amir.





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux