Thread (11 messages) 11 messages, 5 authors, 2020-09-11

RE: [PATCH] vfs: add fchmodat2 syscall

From: David Laight <hidden>
Date: 2020-09-10 19:47:23
Also in: linux-fsdevel, lkml

From: Rich Felker
Sent: 10 September 2020 15:24
...
quoted hunk ↗ jump to hunk
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)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help