Re: [PATCH] fs/stat: reject invalid flags for empty paths

[Jan Kara]

On Sat 31-10-26 09:58:10, Avi Weiss wrote:
> vfs_fstatat() and statx() take the file descriptor fast path when
> AT_EMPTY_PATH is set, even if unsupported flag bits are present.
> This bypasses vfs_statx() flag validation and lets both syscalls
> succeed with AT_RECURSIVE.
> 
> Restrict the fast paths to supported flags so invalid combinations
> fall through to vfs_statx() and return -EINVAL.
> 
> Fixes: e896474fe485 ("getname_maybe_null() - the third variant of pathname copy-in")
> Signed-off-by: Avi Weiss <thnkslprpt@xxxxxxxxx>
> ---
>  fs/stat.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/stat.c b/fs/stat.c
> index a2e63c6c45b7..b75a411b502f 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -371,7 +371,9 @@ int vfs_fstatat(int dfd, const char __user *filename,
>  {
>  	CLASS(filename_maybe_null, name)(filename, flags);
>  
> -	if (!name && dfd >= 0)
> +	if (!name && dfd >= 0 &&
> +	    !(flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT |
> +			AT_EMPTY_PATH | AT_STATX_SYNC_TYPE)))
>  		return vfs_fstat(dfd, stat);

Shouldn't we just return EINVAL in the case of unsupported flag instead of
falling back to vfs_statx() to fail?

>  	return vfs_statx(dfd, name, flags | AT_NO_AUTOMOUNT,
> @@ -812,7 +814,9 @@ SYSCALL_DEFINE5(statx,
>  {
>  	CLASS(filename_maybe_null, name)(filename, flags);
>  
> -	if (!name && dfd >= 0)
> +	if (!name && dfd >= 0 &&
> +	    !(flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT |
> +			AT_EMPTY_PATH | AT_STATX_SYNC_TYPE)))
>  		return do_statx_fd(dfd, flags & ~AT_NO_AUTOMOUNT, mask, buffer);

For symmetry this belongs to vfs_statx_fd() similarly as in the other path
the checks are done in vfs_statx().

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR