[PATCH] xfs: warn about inodes with project id of -1

[PATCH] xfs: warn about inodes with project id of -1

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Inodes aren't supposed to have a project id of -1U (aka 4294967295) but
the kernel hasn't always validated FSSETXATTR correctly.  Flag this as
something for the sysadmin to check out.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/scrub/inode.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
index 2405b09d03d0..eac15af7b08c 100644
--- a/fs/xfs/scrub/inode.c
+++ b/fs/xfs/scrub/inode.c
@@ -233,6 +233,7 @@ xchk_dinode(
 	unsigned long long	isize;
 	uint64_t		flags2;
 	uint32_t		nextents;
+	prid_t			prid;
 	uint16_t		flags;
 	uint16_t		mode;
 
@@ -267,6 +268,7 @@ xchk_dinode(
 		 * so just mark this inode for preening.
 		 */
 		xchk_ino_set_preen(sc, ino);
+		prid = 0;
 		break;
 	case 2:
 	case 3:
@@ -279,12 +281,17 @@ xchk_dinode(
 		if (dip->di_projid_hi != 0 &&
 		    !xfs_has_projid32(mp))
 			xchk_ino_set_corrupt(sc, ino);
+
+		prid = be16_to_cpu(dip->di_projid_lo);
 		break;
 	default:
 		xchk_ino_set_corrupt(sc, ino);
 		return;
 	}
 
+	if (xfs_has_projid32(mp))
+		prid |= (prid_t)be16_to_cpu(dip->di_projid_hi) << 16;
+
 	/*
 	 * di_uid/di_gid -- -1 isn't invalid, but there's no way that
 	 * userspace could have created that.
@@ -293,6 +300,13 @@ xchk_dinode(
 	    dip->di_gid == cpu_to_be32(-1U))
 		xchk_ino_set_warning(sc, ino);
 
+	/*
+	 * project id of -1 isn't supposed to be valid, but the kernel didn't
+	 * always validate that.
+	 */
+	if (prid == -1U)
+		xchk_ino_set_warning(sc, ino);
+
 	/* di_format */
 	switch (dip->di_format) {
 	case XFS_DINODE_FMT_DEV:

Re: [PATCH] xfs: warn about inodes with project id of -1

[Dave Chinner]

On Tue, Jan 04, 2022 at 03:43:25PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Inodes aren't supposed to have a project id of -1U (aka 4294967295) but
> the kernel hasn't always validated FSSETXATTR correctly.  Flag this as
> something for the sysadmin to check out.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/scrub/inode.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> index 2405b09d03d0..eac15af7b08c 100644
> --- a/fs/xfs/scrub/inode.c
> +++ b/fs/xfs/scrub/inode.c
> @@ -233,6 +233,7 @@ xchk_dinode(
>  	unsigned long long	isize;
>  	uint64_t		flags2;
>  	uint32_t		nextents;
> +	prid_t			prid;
>  	uint16_t		flags;
>  	uint16_t		mode;
>  
> @@ -267,6 +268,7 @@ xchk_dinode(
>  		 * so just mark this inode for preening.
>  		 */
>  		xchk_ino_set_preen(sc, ino);
> +		prid = 0;
>  		break;
>  	case 2:
>  	case 3:
> @@ -279,12 +281,17 @@ xchk_dinode(
>  		if (dip->di_projid_hi != 0 &&
>  		    !xfs_has_projid32(mp))
>  			xchk_ino_set_corrupt(sc, ino);
> +
> +		prid = be16_to_cpu(dip->di_projid_lo);
>  		break;
>  	default:
>  		xchk_ino_set_corrupt(sc, ino);
>  		return;
>  	}
>  
> +	if (xfs_has_projid32(mp))
> +		prid |= (prid_t)be16_to_cpu(dip->di_projid_hi) << 16;
> +
>  	/*
>  	 * di_uid/di_gid -- -1 isn't invalid, but there's no way that
>  	 * userspace could have created that.
> @@ -293,6 +300,13 @@ xchk_dinode(
>  	    dip->di_gid == cpu_to_be32(-1U))
>  		xchk_ino_set_warning(sc, ino);
>  
> +	/*
> +	 * project id of -1 isn't supposed to be valid, but the kernel didn't
> +	 * always validate that.
> +	 */
> +	if (prid == -1U)
> +		xchk_ino_set_warning(sc, ino);
> +
>  	/* di_format */
>  	switch (dip->di_format) {
>  	case XFS_DINODE_FMT_DEV:

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com