Re: [PATCH] vfs: add faccessat2 syscall

From: Florian Weimer
Date: Thu Apr 16 2020 - 10:43:57 EST


* Miklos Szeredi:

> POSIX defines faccessat() as having a fourth "flags" argument, while the
> linux syscall doesn't have it. Glibc tries to emulate AT_EACCESS and
> AT_SYMLINK_NOFOLLOW, but AT_EACCESS emulation is broken.
>
> Add a new faccessat(2) syscall with the added flags argument and implement
> both flags.
>
> The value of AT_EACCESS is defined in glibc headers to be the same as
> AT_REMOVEDIR. Use this value for the kernel interface as well, together
> with the explanatory comment.

Thanks a lot for this!

> +long do_faccessat(int dfd, const char __user *filename, int mode, int flags)
> +{
> + const struct cred *old_cred = NULL;
> + struct path path;
> + struct inode *inode;
> + int res;
> + unsigned int lookup_flags = LOOKUP_FOLLOW;
> +
> + if (mode & ~S_IRWXO) /* where's F_OK, X_OK, W_OK, R_OK? */
> + return -EINVAL;
> +
> + if (flags & ~(AT_EACCESS | AT_SYMLINK_NOFOLLOW))
> + return -EINVAL;

Should this accept AT_EMPTY_PATH as well?

(I can't comment on the rest of the logic of the patch.)

> diff --git a/include/uapi/linux/fcntl.h b/include/uapi/linux/fcntl.h
> index ca88b7bce553..2f86b2ad6d7e 100644
> --- a/include/uapi/linux/fcntl.h
> +++ b/include/uapi/linux/fcntl.h
> @@ -84,10 +84,20 @@
> #define DN_ATTRIB 0x00000020 /* File changed attibutes */
> #define DN_MULTISHOT 0x80000000 /* Don't remove notifier */
>
> +/*
> + * The constants AT_REMOVEDIR and AT_EACCESS have the same value. AT_EACCESS is
> + * meaningful only to faccessat, while AT_REMOVEDIR is meaningful only to
> + * unlinkat. The two functions do completely different things and therefore,
> + * the flags can be allowed to overlap. For example, passing AT_REMOVEDIR to
> + * faccessat would be undefined behavior and thus treating it equivalent to
> + * AT_EACCESS is valid undefined behavior.
> + */
> #define AT_FDCWD -100 /* Special value used to indicate
> openat should use the current
> working directory. */
> #define AT_SYMLINK_NOFOLLOW 0x100 /* Do not follow symbolic links. */
> +#define AT_EACCESS 0x200 /* Test access permitted for
> + effective IDs, not real IDs. */
> #define AT_REMOVEDIR 0x200 /* Remove directory instead of
> unlinking file. */
> #define AT_SYMLINK_FOLLOW 0x400 /* Follow symbolic links. */

I can confirm that this is what glibc does, ofr better or worse.

Thanks,
Florian