linux-ext4 - Re: fstest generic/696 failure on ext4 fs with quotas+idmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87il2ghage.fsf@suse.de>
Date: Thu, 22 Feb 2024 13:53:21 +0000
From: Luis Henriques <lhenriques@...e.de>
To: Christian Brauner <brauner@...nel.org>
Cc: linux-ext4@...r.kernel.org
Subject: Re: fstest generic/696 failure on ext4 fs with quotas+idmap

Christian Brauner <brauner@...nel.org> writes:

> 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.

Wow! Awesome, thanks a lot for looking into this, Christian.  I'll test
that patch, but from your description it looks like it should be fix.

Cheers,
-- 
Luís