Received-SPF: pass (google.com: domain of linux-ext4+bounces-1359-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 22 Feb 2024 14:26:27 +0100
From: Christian Brauner <brauner@kernel.org>
To: Luis Henriques <lhenriques@suse.de>
Cc: linux-ext4@vger.kernel.org
Subject: Re: fstest generic/696 failure on ext4 fs with quotas+idmap
Message-ID: <20240222-knast-reifen-953312ce17a9@brauner>
References: <87jzmxisqm.fsf@suse.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <87jzmxisqm.fsf@suse.de>

On Wed, Feb 21, 2024 at 06:20:49PM +0000, Luis Henriques wrote:
> Hi!
> 
> The fstest generic/696 (and 697) fail on ext4 when the filesystem is
> created with quota support (-O quota).  It's really easy to reproduce, and
> it fails when doing the idmapped tests (setgid_create_umask_idmapped() and
> setgid_create_umask_idmapped_in_userns()).
> 
> The failure happens when the test does an openat() with O_TMPFILE:
> 
>   ext4_tmpfile()
>     __ext4_new_inode()
>       dquot_initialize()
>         dqget()
> 
> and at this point the error occurs:
> 
> 	if (!qid_has_mapping(sb->s_user_ns, qid))
> 		return ERR_PTR(-EINVAL);
> 
> qid is '-1', which is invalid, but I'm failing to understand if it should
> really be invalid or if dqget() should handle this invalid qid some other
> way.  Earlier, __ext4_new_inode() called inode_init_owner(), which indeed
> sets inode->i_uid with '-1'.

I think that's a misanalysis? The dquot_initialize happens to be before
the inode creation for that tmpfile. Anyway, see below.

> 
> I've been trying to figure it out, but it's very tricky to follow all the
> details, so I decided to ask here and see if anyone has any idea.  Is this
> a known issue?  Maybe the issue is with the test itself, and not with
> ext4, quota or idmapped code.

So good new is that it's neither an ext4, quota, or idmapped bug. It's
just the test being broken because openat_tmpfile_supported() is called
after we created an idmapped mount on the idmapped mount which means
that the callers fs{g,u}id might not be mapped. That means
make_kquid_*id() will return INVALID_*ID which will later fail that
check whether the qid is mapped in dqget().

I sent a patch to xfstests with you can ext4 Cced. I've tested it here
and it's fixed. Feel free to test as well.