On Mon, Mar 04, 2024 at 04:50:14PM +0100, Andrey Albershteyn wrote:
> There's an issue that if special files is created before quota
> project is enabled, then it's not possible to link this file. This
> works fine for normal files. This happens because xfs_quota skips
> special files (no ioctls to set necessary flags). The check for
> having the same project ID for source and destination then fails as
> source file doesn't have any ID.
>
> mkfs.xfs -f /dev/sda
> mount -o prjquota /dev/sda /mnt/test
>
> mkdir /mnt/test/foo
> mkfifo /mnt/test/foo/fifo1
>
> xfs_quota -xc "project -sp /mnt/test/foo 9" /mnt/test
> > Setting up project 9 (path /mnt/test/foo)...
> > xfs_quota: skipping special file /mnt/test/foo/fifo1
> > Processed 1 (/etc/projects and cmdline) paths for project 9 with recursion depth infinite (-1).
>
> ln /mnt/test/foo/fifo1 /mnt/test/foo/fifo1_link
> > ln: failed to create hard link '/mnt/test/testdir/fifo1_link' => '/mnt/test/testdir/fifo1': Invalid cross-device link
>
> mkfifo /mnt/test/foo/fifo2
> ln /mnt/test/foo/fifo2 /mnt/test/foo/fifo2_link
>
> Fix this by allowing linking of special files to the project quota
> if special files doesn't have any ID set (ID = 0).
Reasonable.
> Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> ---
> fs/xfs/xfs_inode.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 5ca561634164..641270f4d794 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1232,11 +1232,24 @@ xfs_link(
> * the tree quota mechanism could be circumvented.
> */
> if (unlikely((tdp->i_diflags & XFS_DIFLAG_PROJINHERIT) &&
> + !special_file(VFS_I(sip)->i_mode) &&
> tdp->i_projid != sip->i_projid)) {
> error = -EXDEV;
> goto error_return;
> }
>
> + /*
> + * Don't allow cross-linking of special files. However, allow
> + * cross-linking if original file doesn't have any project.
> + */
The comment should explain why the code exists, not document what
the code does.
> + if (unlikely((tdp->i_diflags & XFS_DIFLAG_PROJINHERIT) &&
> + special_file(VFS_I(sip)->i_mode) &&
> + sip->i_projid != 0 &&
> + tdp->i_projid != sip->i_projid)) {
> + error = -EXDEV;
> + goto error_return;
> + }
I think this would be better written as:
/*
* If we are using project inheritance, we only allow hard link
* creation in our tree when the project IDs are the same; else
* the tree quota mechanism could be circumvented.
*/
if ((tdp->i_diflags & XFS_DIFLAG_PROJINHERIT) &&
tdp->i_projid != sip->i_projid) {
/*
* Project quota setup skips special files which can
* leave inodes in a PROJINHERIT directory without a
* project ID set. We need to allow links to be made
* to these "project-less" inodes because userspace
* expects them to succeed after project ID setup,
* but everything else should be rejected.
*/
if (!special_file(VFS_I(sip)->i_mode) ||
sip->i_projid != 0) {
error = -EXDEV;
goto error_return;
}
}
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
