Re: [PATCH] xfs: honor init_xattrs in xfs_init_new_inode for !attr && attr2 fs

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, Jun 18, 2024 at 04:21:12PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> xfs_init_new_inode ignores the init_xattrs parameter for filesystems
> that have ATTR2 enabled but not ATTR.  As a result, the first file to be
> created by the kernel will not have an attr fork created to store acls.
> Storing that first acl will add ATTR to the superblock flags, so chances
> are nobody has noticed this previously.
> 
> However, this is disastrous on a filesystem with parent pointers because
> it requires that a new linkable file /must/ have a pre-existing attr
> fork.  The preproduction version of mkfs.xfs have always set this, but
> as xfs_sb.c doesn't validate that pptrs filesystems have ATTR set, we
> must catch this case.
> 
> Note that the sb verifier /does/ require ATTR2 for V5 filesystems, so we
> can fix both problems by amending xfs_init_new_inode to set up the attr
> fork for either ATTR or ATTR2.
> 
> Fixes: 2442ee15bb1e ("xfs: eager inode attr fork init needs attr feature awareness")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  fs/xfs/xfs_inode.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index b699fa6ee3b64..b2aab91a86d30 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -870,7 +870,7 @@ xfs_init_new_inode(
>  	 * this saves us from needing to run a separate transaction to set the
>  	 * fork offset in the immediate future.
>  	 */
> -	if (init_xattrs && xfs_has_attr(mp)) {
> +	if (init_xattrs && (xfs_has_attr(mp) || xfs_has_attr2(mp))) {

NAK, this patch is still not correct -- if we add an attr fork here, we
also have to xfs_add_attr().  ATTR protects attr forks in general,
whereas ATTR2 only protects dynamic fork sizes.

	if (init_xattrs && (xfs_has_attr(mp) || xfs_has_attr2(mp))) {
		ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
		xfs_ifork_init_attr(ip, XFS_DINODE_FMT_EXTENTS, 0);

		if (!xfs_has_attr(mp)) {
			spin_lock(&mp->m_sb_lock);
			xfs_add_attr2(mp);
			spin_unlock(&mp->m_sb_lock);
			xfs_log_sb(tp);
		}
	}

--D

>  		ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
>  		xfs_ifork_init_attr(ip, XFS_DINODE_FMT_EXTENTS, 0);
>  	}
>