xfs_io: additions for testing copy_range

From: Dave Chinner <[email protected]>

So that O_RDONLY open commands (such as from copy_range) do not block forever waiting on a non-existent writer.

Signed-off-by Dave Chinner <[email protected]>

---
 io/open.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

io/open.c +13 -1

index 6ea3e9a2019f..b1d9a0fa317c 100644
@@ -56,6 +56,7 @@ openfile(
        struct fs_path  *fs_path)
 {
        struct fs_path  *fsp;
+       struct stat     st;
        int             fd;
        int             oflags;
 

@@ -79,6 +80,18 @@ openfile(
        if (flags & IO_NOFOLLOW)
                oflags |= O_NOFOLLOW;
 
+       /*
+        * if we've been passed a pipe to open, don't block waiting for a
+        * reader or writer to appear. We want to either succeed or error out
+        * immediately.
+        */
+       if (stat(path, &st) < 0 && errno != ENOENT) {
+               perror("stat");
+               return -1;
+       }
+       if (S_ISFIFO(st.st_mode))
+               oflags |= O_NONBLOCK;
+
        fd = open(path, oflags, mode);
        if (fd < 0) {
                if (errno == EISDIR &&
--

2.19.1

From: Dave Chinner <[email protected]>

So that O_RDONLY open commands (such as from copy_range) do not block forever waiting on a non-existent writer.

Signed-off-by Dave Chinner <[email protected]>

---
 io/open.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

io/open.c +1 -0

index 6ea3e9a2019f..b1d9a0fa317c 100644
@@ -56,6 +56,7 @@ openfile(
 	struct fs_path	*fs_path)
 {
 	struct fs_path	*fsp;
+	struct stat	st;
 	int		fd;
 	int		oflags;
 

@@ -79,6 +80,18 @@ openfile(
 	if (flags & IO_NOFOLLOW)
 		oflags |= O_NOFOLLOW;
 

io/open.c +2 -0

+		oflags |= O_NONBLOCK;
+
 	fd = open(path, oflags, mode);
 	if (fd < 0) {
 		if (errno == EISDIR &&

Signature · 1 line

2.19.1

Quoted 26 lines · Dave Chinner

From: Dave Chinner <[email protected]>

So that O_RDONLY open commands (such as from copy_range) do not block forever waiting on a non-existent writer.

Signed-off-by Dave Chinner <[email protected]>

---
 io/open.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

io/open.c +1 -0

index 6ea3e9a2019f..b1d9a0fa317c 100644
@@ -56,6 +56,7 @@ openfile(
 	struct fs_path	*fs_path)
 {
 	struct fs_path	*fsp;
+	struct stat	st;
 	int		fd;
 	int		oflags;
 

@@ -79,6 +80,18 @@ openfile(
 	if (flags & IO_NOFOLLOW)
 		oflags |= O_NOFOLLOW;
 

io/open.c openfile( · line 92

Quoted 10 lines

io/open.c +10 -0

+	/*
+	 * if we've been passed a pipe to open, don't block waiting for a
+	 * reader or writer to appear. We want to either succeed or error out
+	 * immediately.
+	 */
+	if (stat(path, &st) < 0 && errno != ENOENT) {
+		perror("stat");
+		return -1;
+	}
+	if (S_ISFIFO(st.st_mode))

Hmm... does stat() set st.st_mode even if it returns ENOENT? Or are we possibly branching on an uninitialized st here?

struct stat st = { 0 }; ?

Quoted 41 lines · Darrick J. Wong

Quoted 26 lines · Dave Chinner

From: Dave Chinner <[email protected]>

So that O_RDONLY open commands (such as from copy_range) do not block forever waiting on a non-existent writer.

Signed-off-by Dave Chinner <[email protected]>

---
 io/open.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

io/open.c +1 -0

index 6ea3e9a2019f..b1d9a0fa317c 100644
@@ -56,6 +56,7 @@ openfile(
 	struct fs_path	*fs_path)
 {
 	struct fs_path	*fsp;
+	struct stat	st;
 	int		fd;
 	int		oflags;
 

@@ -79,6 +80,18 @@ openfile(
 	if (flags & IO_NOFOLLOW)
 		oflags |= O_NOFOLLOW;
 

io/open.c openfile( · line 92

Quoted 10 lines

io/open.c +10 -0

+	/*
+	 * if we've been passed a pipe to open, don't block waiting for a
+	 * reader or writer to appear. We want to either succeed or error out
+	 * immediately.
+	 */
+	if (stat(path, &st) < 0 && errno != ENOENT) {
+		perror("stat");
+		return -1;
+	}
+	if (S_ISFIFO(st.st_mode))

Hmm... does stat() set st.st_mode even if it returns ENOENT? Or are we possibly branching on an uninitialized st here?

Quoted 1 line

struct stat st = { 0 }; ?

Yeah, I'll fix that.

From: Dave Chinner <[email protected]>

I need to be able to open a file read-write, then change the permissions on the file to read-only to check that copy_file_range returns EPERM correctly in that case. This can't be done as root, because root ignores file permissions, but as a normal user we can't open a 0444 file for writing and so can't actually test writing to a read-only file without some method of "open read-write, change permissions to read-only, try to write to file through open read-write file".

So, allow adding or removing write permissions on an open file.

Signed-off-by Dave Chinner <[email protected]>

---
 io/open.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

io/open.c +54 -1

index b1d9a0fa317c..153d4a836d4c 100644
@@ -44,6 +44,7 @@ static cmdinfo_t chproj_cmd;
 static cmdinfo_t lsproj_cmd;
 static cmdinfo_t extsize_cmd;
 static cmdinfo_t inode_cmd;
+static cmdinfo_t chmod_cmd;
 static prid_t prid;
 static long extsize;
 

@@ -809,6 +810,48 @@ inode_f(
        return 0;
 }
 
+static void
+chmod_help(void)
+{
+       printf(_(
+"\n"
+" Change the read/write permissions on the current file\n"
+"\n"
+" Options:\n"
+" -r -- make the file read only (0444 permissions)\n"
+" -w -- make the file read/write (0664 permissions)\n"
+"\n"));
+}
+
+static int
+chmod_f(
+       int             argc,
+       char            **argv)
+{
+       mode_t          mode = S_IRUSR | S_IRGRP | S_IROTH;
+       int             c;
+
+       while ((c = getopt(argc, argv, "rw")) != EOF) {
+               switch (c) {
+               case 'r':
+                       break;
+               case 'w':
+                       mode |= S_IWUSR | S_IWGRP;
+                       break;
+               default:
+                       return command_usage(&chmod_cmd);
+               }
+       }
+
+       if (argc != optind)
+               return command_usage(&chmod_cmd);
+
+       if (fchmod(file->fd, mode) < 0) {
+               exitcode = 1;
+               perror("fchmod");
+       }
+       return 0;
+}
 void
 open_init(void)
 {

@@ -871,10 +914,21 @@ open_init(void)
                _("Query inode number usage in the filesystem");
        inode_cmd.help = inode_help;
 
+       chmod_cmd.name = "chmod";
+       chmod_cmd.cfunc = chmod_f;
+       chmod_cmd.args = _("-r | -w");
+       chmod_cmd.argmin = 1;
+       chmod_cmd.argmax = 1;
+       chmod_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
+       chmod_cmd.oneline =
+               _("change the read/write permissios on the currently open file");
+       chmod_cmd.help = chmod_help;
+
        add_command(&open_cmd);
        add_command(&close_cmd);
        add_command(&chproj_cmd);
        add_command(&lsproj_cmd);
        add_command(&extsize_cmd);
        add_command(&inode_cmd);
+       add_command(&chmod_cmd);
 }
--

2.19.1

Quoted 12 lines · Dave Chinner

From: Dave Chinner <[email protected]>

I need to be able to open a file read-write, then change the permissions on the file to read-only to check that copy_file_range returns EPERM correctly in that case. This can't be done as root, because root ignores file permissions, but as a normal user we can't open a 0444 file for writing and so can't actually test writing to a read-only file without some method of "open read-write, change permissions to read-only, try to write to file through open read-write file".

Quoted 3 lines

So, allow adding or removing write permissions on an open file.

Signed-off-by Dave Chinner <[email protected]>

Should there be a man page update as well? Other commands from open.c have a corresponding man section, but:

$ grep -c chmod man/man8/xfs_io.8
0

And I wonder if the two permissions (0444 and 0664) are enough, or we might want to add other modes as well. But maybe that can be added when the need comes... Otherwise it looks good.

From: Dave Chinner <[email protected]>

I need to be able to open a file read-write, then change the permissions on the file to read-only to check that copy_file_range returns EPERM correctly in that case. This can't be done as root, because root ignores file permissions, but as a normal user we can't open a 0444 file for writing and so can't actually test writing to a read-only file without some method of "open read-write, change permissions to read-only, try to write to file through open read-write file".

So, allow adding or removing write permissions on an open file.

Signed-off-by Dave Chinner <[email protected]>

---
 io/open.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

io/open.c +43 -0

index b1d9a0fa317c..153d4a836d4c 100644
@@ -44,6 +44,7 @@ static cmdinfo_t chproj_cmd;
 static cmdinfo_t lsproj_cmd;
 static cmdinfo_t extsize_cmd;
 static cmdinfo_t inode_cmd;
+static cmdinfo_t chmod_cmd;
 static prid_t prid;
 static long extsize;
 

@@ -809,6 +810,48 @@ inode_f(
 	return 0;
 }
 
+static void
+chmod_help(void)
+{
+	printf(_(
+"\n"
+" Change the read/write permissions on the current file\n"
+"\n"
+" Options:\n"
+" -r -- make the file read only (0444 permissions)\n"
+" -w -- make the file read/write (0664 permissions)\n"
+"\n"));
+}
+
+static int
+chmod_f(
+	int		argc,
+	char		**argv)
+{
+	mode_t		mode = S_IRUSR | S_IRGRP | S_IROTH;
+	int		c;
+
+	while ((c = getopt(argc, argv, "rw")) != EOF) {
+		switch (c) {
+		case 'r':
+			break;
+		case 'w':
+			mode |= S_IWUSR | S_IWGRP;
+			break;
+		default:
+			return command_usage(&chmod_cmd);
+		}
+	}
+
+	if (argc != optind)
+		return command_usage(&chmod_cmd);
+
+	if (fchmod(file->fd, mode) < 0) {
+		exitcode = 1;
+		perror("fchmod");
+	}
+	return 0;
+}
 void
 open_init(void)
 {

@@ -871,10 +914,21 @@ open_init(void)
 		_("Query inode number usage in the filesystem");

io/open.c +3 -0

+	chmod_cmd.help = chmod_help;
+
 	add_command(&open_cmd);
 	add_command(&close_cmd);
 	add_command(&chproj_cmd);
 	add_command(&lsproj_cmd);
 	add_command(&extsize_cmd);
 	add_command(&inode_cmd);
+	add_command(&chmod_cmd);
 }

Signature · 1 line

2.19.1

From: Dave Chinner <[email protected]>

I need to be able to open a file read-write, then change the permissions on the file to read-only to check that copy_file_range returns EPERM correctly in that case. This can't be done as root, because root ignores file permissions, but as a normal user we can't open a 0444 file for writing and so can't actually test writing to a read-only file without some method of "open read-write, change permissions to read-only, try to write to file through open read-write file".

So, allow adding or removing write permissions on an open file.

Signed-off-by Dave Chinner <[email protected]>

---
 io/open.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

io/open.c +54 -0

index b1d9a0fa317c..153d4a836d4c 100644
@@ -44,6 +44,7 @@ static cmdinfo_t chproj_cmd;
 static cmdinfo_t lsproj_cmd;
 static cmdinfo_t extsize_cmd;
 static cmdinfo_t inode_cmd;
+static cmdinfo_t chmod_cmd;
 static prid_t prid;
 static long extsize;
 

@@ -809,6 +810,48 @@ inode_f(
 	return 0;
 }
 
+static void
+chmod_help(void)
+{
+	printf(_(
+"\n"
+" Change the read/write permissions on the current file\n"
+"\n"
+" Options:\n"
+" -r -- make the file read only (0444 permissions)\n"
+" -w -- make the file read/write (0664 permissions)\n"
+"\n"));
+}
+
+static int
+chmod_f(
+	int		argc,
+	char		**argv)
+{
+	mode_t		mode = S_IRUSR | S_IRGRP | S_IROTH;
+	int		c;
+
+	while ((c = getopt(argc, argv, "rw")) != EOF) {
+		switch (c) {
+		case 'r':
+			break;
+		case 'w':
+			mode |= S_IWUSR | S_IWGRP;
+			break;
+		default:
+			return command_usage(&chmod_cmd);
+		}
+	}
+
+	if (argc != optind)
+		return command_usage(&chmod_cmd);
+
+	if (fchmod(file->fd, mode) < 0) {
+		exitcode = 1;
+		perror("fchmod");
+	}
+	return 0;
+}
 void
 open_init(void)
 {

@@ -871,10 +914,21 @@ open_init(void)
 		_("Query inode number usage in the filesystem");
 	inode_cmd.help = inode_help;
 
+	chmod_cmd.name = "chmod";
+	chmod_cmd.cfunc = chmod_f;
+	chmod_cmd.args = _("-r | -w");
+	chmod_cmd.argmin = 1;
+	chmod_cmd.argmax = 1;
+	chmod_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
+	chmod_cmd.oneline =
+		_("change the read/write permissios on the currently open file");
+	chmod_cmd.help = chmod_help;
+
 	add_command(&open_cmd);
 	add_command(&close_cmd);
 	add_command(&chproj_cmd);
 	add_command(&lsproj_cmd);
 	add_command(&extsize_cmd);
 	add_command(&inode_cmd);
+	add_command(&chmod_cmd);
 }

From: Dave Chinner <[email protected]>

I need to be able to open a file read-write, then change the permissions on the file to read-only to check that copy_file_range returns EPERM correctly in that case. This can't be done as root, because root ignores file permissions, but as a normal user we can't open a 0444 file for writing and so can't actually test writing to a read-only file without some method of "open read-write, change permissions to read-only, try to write to file through open read-write file".

So, allow adding or removing write permissions on an open file.

Signed-off-by Dave Chinner <[email protected]>

---
Version 2:
- include a man page update from the new function

 io/open.c         | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/xfs_io.8 |  9 +++++++++
 2 files changed, 63 insertions(+)

io/open.c +54 -0

index 21c0e054f8d2..2663c38e9681 100644
@@ -44,6 +44,7 @@ static cmdinfo_t chproj_cmd;
 static cmdinfo_t lsproj_cmd;
 static cmdinfo_t extsize_cmd;
 static cmdinfo_t inode_cmd;
+static cmdinfo_t chmod_cmd;
 static prid_t prid;
 static long extsize;
 

@@ -809,6 +810,48 @@ inode_f(
 	return 0;
 }
 
+static void
+chmod_help(void)
+{
+	printf(_(
+"\n"
+" Change the read/write permissions on the current file\n"
+"\n"
+" Options:\n"
+" -r -- make the file read only (0444 permissions)\n"
+" -w -- make the file read/write (0664 permissions)\n"
+"\n"));
+}
+
+static int
+chmod_f(
+	int		argc,
+	char		**argv)
+{
+	mode_t		mode = S_IRUSR | S_IRGRP | S_IROTH;
+	int		c;
+
+	while ((c = getopt(argc, argv, "rw")) != EOF) {
+		switch (c) {
+		case 'r':
+			break;
+		case 'w':
+			mode |= S_IWUSR | S_IWGRP;
+			break;
+		default:
+			return command_usage(&chmod_cmd);
+		}
+	}
+
+	if (argc != optind)
+		return command_usage(&chmod_cmd);
+
+	if (fchmod(file->fd, mode) < 0) {
+		exitcode = 1;
+		perror("fchmod");
+	}
+	return 0;
+}
 void
 open_init(void)
 {

@@ -871,10 +914,21 @@ open_init(void)
 		_("Query inode number usage in the filesystem");
 	inode_cmd.help = inode_help;
 
+	chmod_cmd.name = "chmod";
+	chmod_cmd.cfunc = chmod_f;
+	chmod_cmd.args = _("-r | -w");
+	chmod_cmd.argmin = 1;
+	chmod_cmd.argmax = 1;
+	chmod_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
+	chmod_cmd.oneline =
+		_("change the read/write permissios on the currently open file");
+	chmod_cmd.help = chmod_help;
+
 	add_command(&open_cmd);
 	add_command(&close_cmd);
 	add_command(&chproj_cmd);
 	add_command(&lsproj_cmd);
 	add_command(&extsize_cmd);
 	add_command(&inode_cmd);
+	add_command(&chmod_cmd);
 }

man/man8/xfs_io.8 +9 -0

index f1099c32de66..68e03b4558b9 100644
@@ -1226,6 +1226,15 @@ characters long.
 .B crc32cselftest
 Test the internal crc32c implementation to make sure that it computes results
 correctly.
+.TP
+.B chmod \-r | \-w
+Change the mode of the currently open file. The
+.B \-r
+option will set the file permissions to read-only (0444), whilst the
+.B \-w
+option will set the file permissions to read-write (0644). This allows xfs_io to
+set up mismatches between the file permissions and the open file descriptor
+read/write mode to exercise permission checks inside various syscalls.
 .SH SEE ALSO
 .BR mkfs.xfs (8),
 .BR xfsctl (3),

From: Dave Chinner <[email protected]>

copy_file_range() takes a size_t as it's length, not a "long long". Therefore we need to be able to pass sizes larger than 8EB to it to be able to test the interface fully and that requires copy_range to accept all values except an explicit error value of "-1LL".

Signed-off-by Dave Chinner <[email protected]>

---
 io/copy_file_range.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

io/copy_file_range.c +3 -3

index f118a3bfd506..4e2969c9ca47 100644
@@ -34,7 +34,7 @@ copy_range_help(void)
  * glibc buffered copy fallback.
  */
 static loff_t
-copy_file_range_cmd(int fd, long long *src, long long *dst, long long len)
+copy_file_range_cmd(int fd, long long *src, long long *dst, size_t len)
 {
        loff_t ret;
 

@@ -78,7 +78,7 @@ copy_range_f(int argc, char **argv)
 {
        long long src = 0;
        long long dst = 0;
-       long long len = 0;
+       size_t len = 0;
        int opt;
        int ret;
        int fd;

@@ -104,7 +104,7 @@ copy_range_f(int argc, char **argv)
                        break;
                case 'l':
                        len = cvtnum(fsblocksize, fssectsize, optarg);
-                       if (len < 0) {
+                       if (len == -1LL) {
                                printf(_("invalid length -- %s\n"), optarg);
                                return 0;
                        }

From: Dave Chinner <[email protected]>

copy_file_range() takes a size_t as it's length, not a "long long". Therefore we need to be able to pass sizes larger than 8EB to it to be able to test the interface fully and that requires copy_range to accept all values except an explicit error value of "-1LL".

Signed-off-by Dave Chinner <[email protected]>

---
 io/copy_file_range.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

io/copy_file_range.c +3 -3

index f118a3bfd506..4e2969c9ca47 100644
@@ -34,7 +34,7 @@ copy_range_help(void)
  * glibc buffered copy fallback.
  */
 static loff_t
-copy_file_range_cmd(int fd, long long *src, long long *dst, long long len)
+copy_file_range_cmd(int fd, long long *src, long long *dst, size_t len)
 {
 	loff_t ret;
 

@@ -78,7 +78,7 @@ copy_range_f(int argc, char **argv)
 {
 	long long src = 0;
 	long long dst = 0;
-	long long len = 0;
+	size_t len = 0;
 	int opt;
 	int ret;
 	int fd;

@@ -104,7 +104,7 @@ copy_range_f(int argc, char **argv)
 			break;
 		case 'l':
 			len = cvtnum(fsblocksize, fssectsize, optarg);
-			if (len < 0) {
+			if (len == -1LL) {
 				printf(_("invalid length -- %s\n"), optarg);
 				return 0;
 			}