From: Rich Felker
> Sent: 10 September 2020 15:24
...
> index 9af548fb841b..570a21f4d81e 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -610,15 +610,30 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd, umode_t, mode)
> return err;
> }
>
> -static int do_fchmodat(int dfd, const char __user *filename, umode_t mode)
> +static int do_fchmodat(int dfd, const char __user *filename, umode_t mode, int flags)
> {
> struct path path;
> int error;
> unsigned int lookup_flags = LOOKUP_FOLLOW;
> +
> + if (flags & AT_SYMLINK_NOFOLLOW)
> + lookup_flags &= ~LOOKUP_FOLLOW;
> + if (flags & ~AT_SYMLINK_NOFOLLOW)
> + return -EINVAL;
I think I'd swap over those two tests.
So unsupported flags are clearly errored.
> retry:
> error = user_path_at(dfd, filename, lookup_flags, &path);
> if (!error) {
> - error = chmod_common(&path, mode);
> + /* Block chmod from getting to fs layer. Ideally the
> + * fs would either allow it or fail with EOPNOTSUPP,
> + * but some are buggy and return an error but change
> + * the mode, which is non-conforming and wrong.
> + * Userspace emulation of AT_SYMLINK_NOFOLLOW in
> + * glibc and musl blocked it too, for same reason. */
> + if (S_ISLNK(path.dentry->d_inode->i_mode)
> + && (flags & AT_SYMLINK_NOFOLLOW))
> + error = -EOPNOTSUPP;
Again swap the order of the tests. I think it reads better as:
if ((flags & AT_SYMLINK_NOFOLLOW)
&& S_ISLNK(path.dentry->d_inode->i_mode))
error = -EOPNOTSUPP;
As well as saving a few clock cycles.
> + else
> + error = chmod_common(&path, mode);
> path_put(&path);
> if (retry_estale(error, lookup_flags)) {
> lookup_flags |= LOOKUP_REVAL;
...
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
