On Tue, Aug 16, 2022 at 08:15:51PM +0800, Su Yue wrote:
> Since kernel commit b27c82e12965 ("attr: port attribute changes to new
> types"), errno should be EOVERFLOW instead of EINVAL if fchownat(2)
> called with {g,u}id whose mapping are not in fs/mount idmappings.
>
> The errno EOVERFLOW causes failure of generic/656:
> ========================================================================
> generic/656 3s ... [failed, exit status 1]- output mismatch (see /root/xfstests-dev/results//generic/656.out.bad)
> --- tests/generic/656.out 2022-01-03 14:18:41.049999153 +0800
> +++ /root/xfstests-dev/results//generic/656.out.bad 2022-08-16 20:09:41.679999992 +0800
> @@ -1,2 +1,4 @@
> QA output created by 656
> Silence is golden
> +idmapped-mounts.c: 7371: setattr_fix_968219708108 - Value too large for defined data type - failure: errno
> +vfstest.c: 1882: run_test - Success - failure: test that setattr works correctly
> ...
> (Run 'diff -u /root/xfstests-dev/tests/generic/656.out /root/xfstests-dev/results//generic/656.out.bad' to see the entire diff)
> Ran: generic/656
> Failures: generic/656
> ========================================================================
>
> As Christian Brauner explained:
> "
> - EINVAL should only be reported because the target {g,u}id_t has no
> mapping in the caller's idmapping, i.e. doesn't yield a valid k{g,u}id_t.
> - EOVERFLOW should be reported because the target k{g,u}id_t doesn't
> have a mapping in the filesystem idmapping or mount idmapping. IOW,
> the filesystem cannot represent the intended value. The mount's
> idmapping is on a par with the filesystem idmapping and thus a failure
> to represent a vfs{g,u}id_t in the filesystem should yield EOVERFLOW.
> "
>
> EOVERFLOW is another reasonable errno so handle it after fchownat(2)
> failed.
>
> Link:https://lore.kernel.org/fstests/20220816103040.gtgg2w75tzpejas5@wittgenstein/T/#t
> Signed-off-by: Su Yue <glass@xxxxxxxxx>
> ---
Looks good,
Reviewed-by: Christian Brauner (Microsoft) <brauner@xxxxxxxxxx>
