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

Jan Kara
2024-05-24T16:11:09Z

On Thu 23-05-24 13:16:48, Andrey Albershteyn wrote:
> On 2024-05-23 09:48:28, Jan Kara wrote:
> > Hi!
> >
> > On Wed 22-05-24 12:45:09, Andrey Albershteyn wrote:
> > > On 2024-05-22 12:00:07, Jan Kara wrote:
> > > > Hello!
> > > >
> > > > On Mon 20-05-24 18:46:21, Andrey Albershteyn 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@redhat.com>
> > > >
> > > > I'd like to understand one thing. Is it practically useful to set project
> > > > IDs for special inodes? There is no significant disk space usage associated
> > > > with them so wrt quotas we are speaking only about the inode itself. So is
> > > > the concern that user could escape inode project quota accounting and
> > > > perform some DoS? Or why do we bother with two new somewhat hairy ioctls
> > > > for something that seems as a small corner case to me?
> > >
> > > So there's few things:
> > > - Quota accounting is missing only some special files. Special files
> > > created after quota project is setup inherit ID from the project
> > > directory.
> > > - For special files created after the project is setup there's no
> > > way to make them project-less. Therefore, creating a new project
> > > over those will fail due to project ID miss match.
> > > - It wasn't possible to hardlink/rename project-less special files
> > > inside a project due to ID miss match. The linking is fixed, and
> > > renaming is worked around in first patch.
> > >
> > > The initial report I got was about second and last point, an
> > > application was failing to create a new project after "restart" and
> > > wasn't able to link special files created beforehand.
> >
> > I see. OK, but wouldn't it then be an easier fix to make sure we *never*
> > inherit project id for special inodes? And make sure inodes with unset
> > project ID don't fail to be linked, renamed, etc...
>
> But then, in set up project, you can cross-link between projects and
> escape quota this way. During linking/renaming if source inode has
> ID but target one doesn't, we won't be able to tell that this link
> is within the project.

Well, I didn't want to charge these special inodes to project quota at all
so "escaping quota" was pretty much what I suggested to do. But my point
was that since the only thing that's really charged for these inodes is the
inodes itself then does this small inaccuracy really matter in practice?
Are we afraid the user is going to fill the filesystem with symlinks?

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR