* fstest generic/696 failure on ext4 fs with quotas+idmap
@ 2024-02-21 18:20 Luis Henriques
2024-02-22 13:23 ` [PATCH] vfs: fix check for tmpfile support Christian Brauner
2024-02-22 13:26 ` fstest generic/696 failure on ext4 fs with quotas+idmap Christian Brauner
0 siblings, 2 replies; 7+ messages in thread
From: Luis Henriques @ 2024-02-21 18:20 UTC (permalink / raw)
To: linux-ext4; +Cc: Christian Brauner
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'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.
Cheers,
--
Luís
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH] vfs: fix check for tmpfile support 2024-02-21 18:20 fstest generic/696 failure on ext4 fs with quotas+idmap Luis Henriques @ 2024-02-22 13:23 ` Christian Brauner 2024-02-22 14:05 ` Luis Henriques 2024-02-22 13:26 ` fstest generic/696 failure on ext4 fs with quotas+idmap Christian Brauner 1 sibling, 1 reply; 7+ messages in thread From: Christian Brauner @ 2024-02-22 13:23 UTC (permalink / raw) To: Luis Henriques, Zorro Lang; +Cc: Christian Brauner, linux-ext4, fstests When ext4 is used with quota support the test fails with EINVAL because it is run after we idmapped the mount. If the caller's fs{g,u}ids aren't mapped then we fail and log a misleading error. Move the checks for tmpfile support right at the beginning of the test in all tests. Reported-by: Luis Henriques <lhenriques@suse.de> Link: https://lore.kernel.org/r/20240222-knast-reifen-953312ce17a9@brauner Signed-off-by: Christian Brauner <brauner@kernel.org> --- src/vfs/idmapped-mounts.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c index 547182fe..e490f3d7 100644 --- a/src/vfs/idmapped-mounts.c +++ b/src/vfs/idmapped-mounts.c @@ -3815,6 +3815,8 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) goto out; } + supported = openat_tmpfile_supported(info->t_dir1_fd); + /* Changing mount properties on a detached mount. */ attr.userns_fd = get_userns_fd(0, 10000, 10000); if (attr.userns_fd < 0) { @@ -3838,8 +3840,6 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) goto out; } - supported = openat_tmpfile_supported(open_tree_fd); - pid = fork(); if (pid < 0) { log_stderr("failure: fork"); @@ -3991,6 +3991,8 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) goto out; } + supported = openat_tmpfile_supported(info->t_dir1_fd); + /* Changing mount properties on a detached mount. */ attr.userns_fd = get_userns_fd(0, 10000, 10000); if (attr.userns_fd < 0) { @@ -4014,8 +4016,6 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) goto out; } - supported = openat_tmpfile_supported(open_tree_fd); - pid = fork(); if (pid < 0) { log_stderr("failure: fork"); @@ -7715,6 +7715,8 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) goto out; } + supported = openat_tmpfile_supported(info->t_dir1_fd); + /* Changing mount properties on a detached mount. */ attr.userns_fd = get_userns_fd(0, 10000, 10000); if (attr.userns_fd < 0) { @@ -7738,8 +7740,6 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) goto out; } - supported = openat_tmpfile_supported(open_tree_fd); - pid = fork(); if (pid < 0) { log_stderr("failure: fork"); @@ -7929,6 +7929,8 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf goto out; } + supported = openat_tmpfile_supported(info->t_dir1_fd); + /* Changing mount properties on a detached mount. */ attr.userns_fd = get_userns_fd(0, 10000, 10000); if (attr.userns_fd < 0) { @@ -7952,8 +7954,6 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf goto out; } - supported = openat_tmpfile_supported(open_tree_fd); - /* * Below we verify that setgid inheritance for a newly created file or * directory works correctly. As part of this we need to verify that @@ -8163,6 +8163,8 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) goto out; } + supported = openat_tmpfile_supported(info->t_dir1_fd); + /* Changing mount properties on a detached mount. */ attr.userns_fd = get_userns_fd(0, 10000, 10000); if (attr.userns_fd < 0) { @@ -8186,8 +8188,6 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) goto out; } - supported = openat_tmpfile_supported(open_tree_fd); - pid = fork(); if (pid < 0) { log_stderr("failure: fork"); @@ -8518,6 +8518,8 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) goto out; } + supported = openat_tmpfile_supported(info->t_dir1_fd); + /* Changing mount properties on a detached mount. */ attr.userns_fd = get_userns_fd(0, 10000, 10000); if (attr.userns_fd < 0) { @@ -8541,8 +8543,6 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) goto out; } - supported = openat_tmpfile_supported(open_tree_fd); - /* * Below we verify that setgid inheritance for a newly created file or * directory works correctly. As part of this we need to verify that -- 2.43.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] vfs: fix check for tmpfile support 2024-02-22 13:23 ` [PATCH] vfs: fix check for tmpfile support Christian Brauner @ 2024-02-22 14:05 ` Luis Henriques 2024-02-25 15:56 ` Zorro Lang 0 siblings, 1 reply; 7+ messages in thread From: Luis Henriques @ 2024-02-22 14:05 UTC (permalink / raw) To: Christian Brauner; +Cc: Zorro Lang, linux-ext4, fstests Christian Brauner <brauner@kernel.org> writes: > When ext4 is used with quota support the test fails with EINVAL because > it is run after we idmapped the mount. If the caller's fs{g,u}ids aren't > mapped then we fail and log a misleading error. Move the checks for > tmpfile support right at the beginning of the test in all tests. > > Reported-by: Luis Henriques <lhenriques@suse.de> > Link: https://lore.kernel.org/r/20240222-knast-reifen-953312ce17a9@brauner > Signed-off-by: Christian Brauner <brauner@kernel.org> FWIW I've just tested this patch and I can confirm it fixes the failures I was seeing in ext4. Again, thanks a lot, Christian. Cheers, -- Luís > --- > src/vfs/idmapped-mounts.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c > index 547182fe..e490f3d7 100644 > --- a/src/vfs/idmapped-mounts.c > +++ b/src/vfs/idmapped-mounts.c > @@ -3815,6 +3815,8 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) > goto out; > } > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > + > /* Changing mount properties on a detached mount. */ > attr.userns_fd = get_userns_fd(0, 10000, 10000); > if (attr.userns_fd < 0) { > @@ -3838,8 +3840,6 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) > goto out; > } > > - supported = openat_tmpfile_supported(open_tree_fd); > - > pid = fork(); > if (pid < 0) { > log_stderr("failure: fork"); > @@ -3991,6 +3991,8 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) > goto out; > } > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > + > /* Changing mount properties on a detached mount. */ > attr.userns_fd = get_userns_fd(0, 10000, 10000); > if (attr.userns_fd < 0) { > @@ -4014,8 +4016,6 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) > goto out; > } > > - supported = openat_tmpfile_supported(open_tree_fd); > - > pid = fork(); > if (pid < 0) { > log_stderr("failure: fork"); > @@ -7715,6 +7715,8 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) > goto out; > } > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > + > /* Changing mount properties on a detached mount. */ > attr.userns_fd = get_userns_fd(0, 10000, 10000); > if (attr.userns_fd < 0) { > @@ -7738,8 +7740,6 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) > goto out; > } > > - supported = openat_tmpfile_supported(open_tree_fd); > - > pid = fork(); > if (pid < 0) { > log_stderr("failure: fork"); > @@ -7929,6 +7929,8 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf > goto out; > } > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > + > /* Changing mount properties on a detached mount. */ > attr.userns_fd = get_userns_fd(0, 10000, 10000); > if (attr.userns_fd < 0) { > @@ -7952,8 +7954,6 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf > goto out; > } > > - supported = openat_tmpfile_supported(open_tree_fd); > - > /* > * Below we verify that setgid inheritance for a newly created file or > * directory works correctly. As part of this we need to verify that > @@ -8163,6 +8163,8 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) > goto out; > } > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > + > /* Changing mount properties on a detached mount. */ > attr.userns_fd = get_userns_fd(0, 10000, 10000); > if (attr.userns_fd < 0) { > @@ -8186,8 +8188,6 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) > goto out; > } > > - supported = openat_tmpfile_supported(open_tree_fd); > - > pid = fork(); > if (pid < 0) { > log_stderr("failure: fork"); > @@ -8518,6 +8518,8 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) > goto out; > } > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > + > /* Changing mount properties on a detached mount. */ > attr.userns_fd = get_userns_fd(0, 10000, 10000); > if (attr.userns_fd < 0) { > @@ -8541,8 +8543,6 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) > goto out; > } > > - supported = openat_tmpfile_supported(open_tree_fd); > - > /* > * Below we verify that setgid inheritance for a newly created file or > * directory works correctly. As part of this we need to verify that > -- > > 2.43.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vfs: fix check for tmpfile support 2024-02-22 14:05 ` Luis Henriques @ 2024-02-25 15:56 ` Zorro Lang 2024-02-26 8:56 ` Luis Henriques 0 siblings, 1 reply; 7+ messages in thread From: Zorro Lang @ 2024-02-25 15:56 UTC (permalink / raw) To: Luis Henriques; +Cc: Christian Brauner, linux-ext4, fstests On Thu, Feb 22, 2024 at 02:05:20PM +0000, Luis Henriques wrote: > Christian Brauner <brauner@kernel.org> writes: > > > When ext4 is used with quota support the test fails with EINVAL because > > it is run after we idmapped the mount. If the caller's fs{g,u}ids aren't > > mapped then we fail and log a misleading error. Move the checks for > > tmpfile support right at the beginning of the test in all tests. > > > > Reported-by: Luis Henriques <lhenriques@suse.de> > > Link: https://lore.kernel.org/r/20240222-knast-reifen-953312ce17a9@brauner > > Signed-off-by: Christian Brauner <brauner@kernel.org> > > FWIW I've just tested this patch and I can confirm it fixes the failures I > was seeing in ext4. Again, thanks a lot, Christian. Thanks for your confirm, I think we can have your "Tested-by" if you don't mind. > > Cheers, > -- > Luís > > > > --- > > src/vfs/idmapped-mounts.c | 24 ++++++++++++------------ > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c > > index 547182fe..e490f3d7 100644 > > --- a/src/vfs/idmapped-mounts.c > > +++ b/src/vfs/idmapped-mounts.c > > @@ -3815,6 +3815,8 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) > > goto out; > > } > > > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > > + > > /* Changing mount properties on a detached mount. */ > > attr.userns_fd = get_userns_fd(0, 10000, 10000); > > if (attr.userns_fd < 0) { > > @@ -3838,8 +3840,6 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) > > goto out; > > } > > > > - supported = openat_tmpfile_supported(open_tree_fd); > > - > > pid = fork(); > > if (pid < 0) { > > log_stderr("failure: fork"); > > @@ -3991,6 +3991,8 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) > > goto out; > > } > > > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > > + > > /* Changing mount properties on a detached mount. */ > > attr.userns_fd = get_userns_fd(0, 10000, 10000); > > if (attr.userns_fd < 0) { > > @@ -4014,8 +4016,6 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) > > goto out; > > } > > > > - supported = openat_tmpfile_supported(open_tree_fd); > > - > > pid = fork(); > > if (pid < 0) { > > log_stderr("failure: fork"); > > @@ -7715,6 +7715,8 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) > > goto out; > > } > > > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > > + > > /* Changing mount properties on a detached mount. */ > > attr.userns_fd = get_userns_fd(0, 10000, 10000); > > if (attr.userns_fd < 0) { > > @@ -7738,8 +7740,6 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) > > goto out; > > } > > > > - supported = openat_tmpfile_supported(open_tree_fd); > > - > > pid = fork(); > > if (pid < 0) { > > log_stderr("failure: fork"); > > @@ -7929,6 +7929,8 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf > > goto out; > > } > > > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > > + > > /* Changing mount properties on a detached mount. */ > > attr.userns_fd = get_userns_fd(0, 10000, 10000); > > if (attr.userns_fd < 0) { > > @@ -7952,8 +7954,6 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf > > goto out; > > } > > > > - supported = openat_tmpfile_supported(open_tree_fd); > > - > > /* > > * Below we verify that setgid inheritance for a newly created file or > > * directory works correctly. As part of this we need to verify that > > @@ -8163,6 +8163,8 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) > > goto out; > > } > > > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > > + > > /* Changing mount properties on a detached mount. */ > > attr.userns_fd = get_userns_fd(0, 10000, 10000); > > if (attr.userns_fd < 0) { > > @@ -8186,8 +8188,6 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) > > goto out; > > } > > > > - supported = openat_tmpfile_supported(open_tree_fd); > > - > > pid = fork(); > > if (pid < 0) { > > log_stderr("failure: fork"); > > @@ -8518,6 +8518,8 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) > > goto out; > > } > > > > + supported = openat_tmpfile_supported(info->t_dir1_fd); > > + > > /* Changing mount properties on a detached mount. */ > > attr.userns_fd = get_userns_fd(0, 10000, 10000); > > if (attr.userns_fd < 0) { > > @@ -8541,8 +8543,6 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) > > goto out; > > } > > > > - supported = openat_tmpfile_supported(open_tree_fd); > > - > > /* > > * Below we verify that setgid inheritance for a newly created file or > > * directory works correctly. As part of this we need to verify that > > -- > > > > 2.43.0 > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vfs: fix check for tmpfile support 2024-02-25 15:56 ` Zorro Lang @ 2024-02-26 8:56 ` Luis Henriques 0 siblings, 0 replies; 7+ messages in thread From: Luis Henriques @ 2024-02-26 8:56 UTC (permalink / raw) To: Zorro Lang; +Cc: Christian Brauner, linux-ext4, fstests Zorro Lang <zlang@redhat.com> writes: > On Thu, Feb 22, 2024 at 02:05:20PM +0000, Luis Henriques wrote: >> Christian Brauner <brauner@kernel.org> writes: >> >> > When ext4 is used with quota support the test fails with EINVAL because >> > it is run after we idmapped the mount. If the caller's fs{g,u}ids aren't >> > mapped then we fail and log a misleading error. Move the checks for >> > tmpfile support right at the beginning of the test in all tests. >> > >> > Reported-by: Luis Henriques <lhenriques@suse.de> >> > Link: https://lore.kernel.org/r/20240222-knast-reifen-953312ce17a9@brauner >> > Signed-off-by: Christian Brauner <brauner@kernel.org> >> >> FWIW I've just tested this patch and I can confirm it fixes the failures I >> was seeing in ext4. Again, thanks a lot, Christian. > > Thanks for your confirm, I think we can have your "Tested-by" if you don't > mind. Ah, yes of course. I should have sent it explicitly. Tested-by: Luis Henriques <lhenriques@suse.de> Cheers, -- Luís >> >> > --- >> > src/vfs/idmapped-mounts.c | 24 ++++++++++++------------ >> > 1 file changed, 12 insertions(+), 12 deletions(-) >> > >> > diff --git a/src/vfs/idmapped-mounts.c b/src/vfs/idmapped-mounts.c >> > index 547182fe..e490f3d7 100644 >> > --- a/src/vfs/idmapped-mounts.c >> > +++ b/src/vfs/idmapped-mounts.c >> > @@ -3815,6 +3815,8 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > + supported = openat_tmpfile_supported(info->t_dir1_fd); >> > + >> > /* Changing mount properties on a detached mount. */ >> > attr.userns_fd = get_userns_fd(0, 10000, 10000); >> > if (attr.userns_fd < 0) { >> > @@ -3838,8 +3840,6 @@ int tcore_setgid_create_idmapped(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > - supported = openat_tmpfile_supported(open_tree_fd); >> > - >> > pid = fork(); >> > if (pid < 0) { >> > log_stderr("failure: fork"); >> > @@ -3991,6 +3991,8 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > + supported = openat_tmpfile_supported(info->t_dir1_fd); >> > + >> > /* Changing mount properties on a detached mount. */ >> > attr.userns_fd = get_userns_fd(0, 10000, 10000); >> > if (attr.userns_fd < 0) { >> > @@ -4014,8 +4016,6 @@ int tcore_setgid_create_idmapped_in_userns(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > - supported = openat_tmpfile_supported(open_tree_fd); >> > - >> > pid = fork(); >> > if (pid < 0) { >> > log_stderr("failure: fork"); >> > @@ -7715,6 +7715,8 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > + supported = openat_tmpfile_supported(info->t_dir1_fd); >> > + >> > /* Changing mount properties on a detached mount. */ >> > attr.userns_fd = get_userns_fd(0, 10000, 10000); >> > if (attr.userns_fd < 0) { >> > @@ -7738,8 +7740,6 @@ static int setgid_create_umask_idmapped(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > - supported = openat_tmpfile_supported(open_tree_fd); >> > - >> > pid = fork(); >> > if (pid < 0) { >> > log_stderr("failure: fork"); >> > @@ -7929,6 +7929,8 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf >> > goto out; >> > } >> > >> > + supported = openat_tmpfile_supported(info->t_dir1_fd); >> > + >> > /* Changing mount properties on a detached mount. */ >> > attr.userns_fd = get_userns_fd(0, 10000, 10000); >> > if (attr.userns_fd < 0) { >> > @@ -7952,8 +7954,6 @@ static int setgid_create_umask_idmapped_in_userns(const struct vfstest_info *inf >> > goto out; >> > } >> > >> > - supported = openat_tmpfile_supported(open_tree_fd); >> > - >> > /* >> > * Below we verify that setgid inheritance for a newly created file or >> > * directory works correctly. As part of this we need to verify that >> > @@ -8163,6 +8163,8 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > + supported = openat_tmpfile_supported(info->t_dir1_fd); >> > + >> > /* Changing mount properties on a detached mount. */ >> > attr.userns_fd = get_userns_fd(0, 10000, 10000); >> > if (attr.userns_fd < 0) { >> > @@ -8186,8 +8188,6 @@ static int setgid_create_acl_idmapped(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > - supported = openat_tmpfile_supported(open_tree_fd); >> > - >> > pid = fork(); >> > if (pid < 0) { >> > log_stderr("failure: fork"); >> > @@ -8518,6 +8518,8 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > + supported = openat_tmpfile_supported(info->t_dir1_fd); >> > + >> > /* Changing mount properties on a detached mount. */ >> > attr.userns_fd = get_userns_fd(0, 10000, 10000); >> > if (attr.userns_fd < 0) { >> > @@ -8541,8 +8543,6 @@ static int setgid_create_acl_idmapped_in_userns(const struct vfstest_info *info) >> > goto out; >> > } >> > >> > - supported = openat_tmpfile_supported(open_tree_fd); >> > - >> > /* >> > * Below we verify that setgid inheritance for a newly created file or >> > * directory works correctly. As part of this we need to verify that >> > -- >> > >> > 2.43.0 >> > >> >> > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: fstest generic/696 failure on ext4 fs with quotas+idmap 2024-02-21 18:20 fstest generic/696 failure on ext4 fs with quotas+idmap Luis Henriques 2024-02-22 13:23 ` [PATCH] vfs: fix check for tmpfile support Christian Brauner @ 2024-02-22 13:26 ` Christian Brauner 2024-02-22 13:53 ` Luis Henriques 1 sibling, 1 reply; 7+ messages in thread From: Christian Brauner @ 2024-02-22 13:26 UTC (permalink / raw) To: Luis Henriques; +Cc: linux-ext4 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: fstest generic/696 failure on ext4 fs with quotas+idmap 2024-02-22 13:26 ` fstest generic/696 failure on ext4 fs with quotas+idmap Christian Brauner @ 2024-02-22 13:53 ` Luis Henriques 0 siblings, 0 replies; 7+ messages in thread From: Luis Henriques @ 2024-02-22 13:53 UTC (permalink / raw) To: Christian Brauner; +Cc: linux-ext4 Christian Brauner <brauner@kernel.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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-02-26 8:56 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-21 18:20 fstest generic/696 failure on ext4 fs with quotas+idmap Luis Henriques 2024-02-22 13:23 ` [PATCH] vfs: fix check for tmpfile support Christian Brauner 2024-02-22 14:05 ` Luis Henriques 2024-02-25 15:56 ` Zorro Lang 2024-02-26 8:56 ` Luis Henriques 2024-02-22 13:26 ` fstest generic/696 failure on ext4 fs with quotas+idmap Christian Brauner 2024-02-22 13:53 ` Luis Henriques
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.