Skip to contentSkip to conversation

xfs_io: Allow "open -P" to specify O_PATH

PATCH In review David Howells 4 participants 9 messages Mar 28, 2017 – Mar 2, 2018 VFS

lore
Share & tools
lore

David Howells

author
#0 lore raw

PATCH 1/1 xfs_io: Allow "open -P" to specify O_PATH

xfs_io: Allow "open -P" to specify O_PATH

Allow "open -P" to specify O_PATH so that paths which would otherwise be unopenable might be opened for stat()'ing. Such things include files that would incur an access error or device files for which no corresponding driver is available.

Signed-off-by David Howells <[email protected]>
---
 io/io.h           |    1 +
 io/open.c         |   13 ++++++++++++-
 man/man8/xfs_io.8 |    6 +++++-
 3 files changed, 18 insertions(+), 2 deletions(-)
io/io.h +1 -0
index 952bdb8..b49a133 100644
@@ -38,6 +38,7 @@
 #define IO_FOREIGN	(1<<7)
 #define IO_NONBLOCK	(1<<8)
 #define IO_TMPFILE	(1<<9)
+#define IO_PATH		(1<<10)
 
 /*
  * Regular file I/O control
io/open.c +12 -1
index 2ed55cf..9eba8ae 100644
@@ -72,6 +72,8 @@ openfile(
 		oflags |= O_NONBLOCK;
 	if (flags & IO_TMPFILE)
 		oflags |= O_TMPFILE;
+	if (flags & IO_PATH)
+		oflags |= O_PATH;
 
 	fd = open(path, oflags, mode);
 	if (fd < 0) {

@@ -179,6 +181,7 @@ open_help(void)
 " -t -- open with O_TRUNC (truncate the file to zero length if it exists)\n"
 " -R -- mark the file as a realtime XFS file immediately after opening it\n"
 " -T -- open with O_TMPFILE (create a file not visible in the namespace)\n"
+" -P -- open with O_PATH (create an fd that is merely a location reference)\n"
 " Note1: usually read/write direct IO requests must be blocksize aligned;\n"
 "        some kernels, however, allow sectorsize alignment for direct IO.\n"
 " Note2: the bmap for non-regular files can be obtained provided the file\n"

@@ -203,7 +206,7 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "FPRTacdfm:nrstx")) != EOF) {
 		switch (c) {
 		case 'F':
 			/* Ignored / deprecated now, handled automatically */

@@ -244,6 +247,9 @@ open_f(
 		case 'T':
 			flags |= IO_TMPFILE;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
 		default:
 			return command_usage(&open_cmd);
 		}

@@ -257,6 +263,11 @@ open_f(
 		return -1;
 	}
 
+	if ((flags & IO_PATH) && (flags & ~IO_PATH)) {
+		fprintf(stderr, _("-P is incompatible with other options\n"));
+		return -1;
+	}
+
 	fd = openfile(argv[optind], &geometry, flags, mode);
 	if (fd < 0)
 		return 0;
man/man8/xfs_io.8 +5 -1
index 022f0ea..77ba760 100644
@@ -122,7 +122,7 @@ command for more details on any command.
 Display a list of all open files and (optionally) switch to an alternate
 current open file.
 .TP
-.BI "open [[ \-acdfrstRT ] " path " ]"
+.BI "open [[ \-acdfrstRTP ] " path " ]"
 Closes the current file, and opens the file specified by
 .I path
 instead. Without any arguments, displays statistics about the current

@@ -164,6 +164,10 @@ option.
 .B \-R
 marks the file as a realtime XFS file after
 opening it, if it is not already marked as such.
+.TP
+.B \-P
+opens the path as a referent only (O_PATH).  This is incompatible with other
+flags specifying other O_xxx flags.
 .PD
 .RE
 .TP
author
#1 lore raw

PATCH 1/1 xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW

Allow "open -P" to specify O_PATH so that paths which would otherwise be unopenable might be opened for stat()'ing. Such things include files that would incur an access error or device files for which no corresponding driver is available.

Allow "-L" to be given in conjunction with O_PATH to specify O_NOFOLLOW also.

Signed-off-by David Howells <[email protected]>
---
 io/io.h           |    2 ++
 io/open.c         |   22 ++++++++++++++++++++--
 man/man8/xfs_io.8 |   12 +++++++++++-
 3 files changed, 33 insertions(+), 3 deletions(-)
io/io.h +2 -0
index 952bdb8..4399419 100644
@@ -38,6 +38,8 @@
 #define IO_FOREIGN	(1<<7)
 #define IO_NONBLOCK	(1<<8)
 #define IO_TMPFILE	(1<<9)
+#define IO_PATH		(1<<10)
+#define IO_NOFOLLOW	(1<<11)
 
 /*
  * Regular file I/O control
io/open.c +20 -2
index 2ed55cf..f7f508a 100644
@@ -72,6 +72,10 @@ openfile(
 		oflags |= O_NONBLOCK;
 	if (flags & IO_TMPFILE)
 		oflags |= O_TMPFILE;
+	if (flags & IO_PATH)
+		oflags |= O_PATH;
+	if (flags & IO_NOFOLLOW)
+		oflags |= O_NOFOLLOW;
 
 	fd = open(path, oflags, mode);
 	if (fd < 0) {

@@ -179,6 +183,8 @@ open_help(void)
 " -t -- open with O_TRUNC (truncate the file to zero length if it exists)\n"
 " -R -- mark the file as a realtime XFS file immediately after opening it\n"
 " -T -- open with O_TMPFILE (create a file not visible in the namespace)\n"
+" -P -- open with O_PATH (create an fd that is merely a location reference)\n"
+" -L -- open with O_NOFOLLOW (don't follow symlink)\n"
 " Note1: usually read/write direct IO requests must be blocksize aligned;\n"
 "        some kernels, however, allow sectorsize alignment for direct IO.\n"
 " Note2: the bmap for non-regular files can be obtained provided the file\n"

@@ -203,7 +209,7 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "FLPRTacdfm:nrstx")) != EOF) {
 		switch (c) {
 		case 'F':
 			/* Ignored / deprecated now, handled automatically */

@@ -244,6 +250,12 @@ open_f(
 		case 'T':
 			flags |= IO_TMPFILE;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
+		case 'L':
+			flags |= IO_NOFOLLOW;
+			break;
 		default:
 			return command_usage(&open_cmd);
 		}

@@ -257,6 +269,12 @@ open_f(
 		return -1;
 	}
 
+	if ((flags & (IO_PATH|IO_NOFOLLOW)) &&
+	    (flags & ~(IO_PATH|IO_NOFOLLOW))) {
+		fprintf(stderr, _("-P and -L are incompatible with the other options\n"));
+		return -1;
+	}
+
 	fd = openfile(argv[optind], &geometry, flags, mode);
 	if (fd < 0)
 		return 0;

@@ -772,7 +790,7 @@ open_init(void)
 	open_cmd.argmax = -1;
 	open_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
 			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
-	open_cmd.args = _("[-acdrstxT] [-m mode] [path]");
+	open_cmd.args = _("[-acdrstxRTPL] [-m mode] [path]");
 	open_cmd.oneline = _("open the file specified by path");
 	open_cmd.help = open_help;
man/man8/xfs_io.8 +11 -1
index 022f0ea..e77be40 100644
@@ -122,7 +122,7 @@ command for more details on any command.
 Display a list of all open files and (optionally) switch to an alternate
 current open file.
 .TP
-.BI "open [[ \-acdfrstRT ] " path " ]"
+.BI "open [[ \-acdfrstRTPL ] " path " ]"
 Closes the current file, and opens the file specified by
 .I path
 instead. Without any arguments, displays statistics about the current

@@ -164,6 +164,16 @@ option.
 .B \-R
 marks the file as a realtime XFS file after
 opening it, if it is not already marked as such.
+.TP
+.B \-P
+opens the path as a referent only (O_PATH).  This is incompatible with other
+flags specifying other O_xxx flags apart from
+.BR \-L .
+.TP
+.B \-L
+doesn't follow symlinks (O_NOFOLLOW).  This is incompatible with other
+flags specifying other O_xxx flags apart from
+.BR \-P .
 .PD
 .RE
 .TP
author
#2 lore raw

The XFS_IOC_FSGEOMETRY ioctl doesn't like fds that are opened O_PATH

Should XFS_IOC_FSGEOMETRY work on fds that are opened O_PATH on an XFS filesystem? Or should xfs_io not call that ioctl on O_PATH fds?

David

#3 lore raw

Re: The XFS_IOC_FSGEOMETRY ioctl doesn't like fds that are opened O_PATH

Quoted 2 lines · David Howells

Should XFS_IOC_FSGEOMETRY work on fds that are opened O_PATH on an XFS filesystem? Or should xfs_io not call that ioctl on O_PATH fds?

Given that read/write don't work under O_PATH, I wouldn't expect ioctls to work either.

I tried to write a two line program to call FSGEOMETRY to see what actually happens but got so bogged down in trying to figure out what magic incantation of C headers one needs to pull in the appropriate structure typedefs for an ioctl I wasn't even trying to call that I gave up.

Seriously, what the f*ck do you have to #include just to be able to use O_PATH?

--D

Quoted 6 lines

David
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#4 lore raw

Re: The XFS_IOC_FSGEOMETRY ioctl doesn't like fds that are opened O_PATH

Quoted 13 lines · Darrick J. Wong
Quoted 2 lines · David Howells

Should XFS_IOC_FSGEOMETRY work on fds that are opened O_PATH on an XFS filesystem? Or should xfs_io not call that ioctl on O_PATH fds?

Given that read/write don't work under O_PATH, I wouldn't expect ioctls to work either.

I tried to write a two line program to call FSGEOMETRY to see what actually happens but got so bogged down in trying to figure out what magic incantation of C headers one needs to pull in the appropriate structure typedefs for an ioctl I wasn't even trying to call that I gave up.

Quoted 4 lines

Seriously, what the f*ck do you have to #include just to be able to use O_PATH?

From open(2):

"The O_DIRECT, O_NOATIME, O_PATH, and O_TMPFILE flags are Linux-specific. One must define _GNU_SOURCE to obtain their definitions."

Also, ioctl() with an O_PATH file descriptor always fails with EBADF because sys_ioctl() uses fdget() rather than fdget_raw(). The filesystem isn't involved at all. This is intentional, I believe.

- Eric

#5 lore raw

Re: The XFS_IOC_FSGEOMETRY ioctl doesn't like fds that are opened O_PATH

Quoted 14 lines · Eric Biggers
Quoted 13 lines · Darrick J. Wong
Quoted 2 lines · David Howells

Should XFS_IOC_FSGEOMETRY work on fds that are opened O_PATH on an XFS filesystem? Or should xfs_io not call that ioctl on O_PATH fds?

Given that read/write don't work under O_PATH, I wouldn't expect ioctls to work either.

I tried to write a two line program to call FSGEOMETRY to see what actually happens but got so bogged down in trying to figure out what magic incantation of C headers one needs to pull in the appropriate structure typedefs for an ioctl I wasn't even trying to call that I gave up.

Quoted 7 lines
Quoted 2 lines

Seriously, what the f*ck do you have to #include just to be able to use O_PATH?

From open(2):

"The O_DIRECT, O_NOATIME, O_PATH, and O_TMPFILE flags are Linux-specific. One must define _GNU_SOURCE to obtain their definitions."

Ah, thank you. I missed that, having skipped straight to the section on O_PATH... ($deity I hate userspace)

Quoted 3 lines

Also, ioctl() with an O_PATH file descriptor always fails with EBADF because sys_ioctl() uses fdget() rather than fdget_raw(). The filesystem isn't involved at all. This is intentional, I believe.

Yes, it is.

--D

Quoted 2 lines

- Eric

author
#6 lore raw

Re: The XFS_IOC_FSGEOMETRY ioctl doesn't like fds that are opened O_PATH

In reply to Eric Biggers
Quoted 3 lines · Eric Biggers

Also, ioctl() with an O_PATH file descriptor always fails with EBADF because sys_ioctl() uses fdget() rather than fdget_raw(). The filesystem isn't involved at all. This is intentional, I believe.

Yeah. Apparently so, though some ioctls might make sense if they're asking about the filesystem.

Anyway, I need to fix xfs_io for that now too if I'm going to use that. I wonder what xfs_io needs the geometry for.

I'm getting the impression xfs_io isn't really the tool I should be using for this.

Dvid

#7 lore raw

Re: [PATCH] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW

In reply to David Howells
Quoted 7 lines · David Howells

Allow "open -P" to specify O_PATH so that paths which would otherwise be unopenable might be opened for stat()'ing. Such things include files that would incur an access error or device files for which no corresponding driver is available.

Allow "-L" to be given in conjunction with O_PATH to specify O_NOFOLLOW also.

Hey David -

Trolling through old patches and finding lots of stuff that fell through the cracks. :( This seems ok, but what is the usecase for it? Was this designed for some particular testcase?

Thanks,
-Eric

Quoted 123 lines
Signed-off-by David Howells <[email protected]>
---
 io/io.h           |    2 ++
 io/open.c         |   22 ++++++++++++++++++++--
 man/man8/xfs_io.8 |   12 +++++++++++-
 3 files changed, 33 insertions(+), 3 deletions(-)
io/io.h +2 -0
index 952bdb8..4399419 100644
@@ -38,6 +38,8 @@
 #define IO_FOREIGN	(1<<7)
 #define IO_NONBLOCK	(1<<8)
 #define IO_TMPFILE	(1<<9)
+#define IO_PATH		(1<<10)
+#define IO_NOFOLLOW	(1<<11)
 
 /*
  * Regular file I/O control
io/open.c +20 -2
index 2ed55cf..f7f508a 100644
@@ -72,6 +72,10 @@ openfile(
 		oflags |= O_NONBLOCK;
 	if (flags & IO_TMPFILE)
 		oflags |= O_TMPFILE;
+	if (flags & IO_PATH)
+		oflags |= O_PATH;
+	if (flags & IO_NOFOLLOW)
+		oflags |= O_NOFOLLOW;
 
 	fd = open(path, oflags, mode);
 	if (fd < 0) {

@@ -179,6 +183,8 @@ open_help(void)
 " -t -- open with O_TRUNC (truncate the file to zero length if it exists)\n"
 " -R -- mark the file as a realtime XFS file immediately after opening it\n"
 " -T -- open with O_TMPFILE (create a file not visible in the namespace)\n"
+" -P -- open with O_PATH (create an fd that is merely a location reference)\n"
+" -L -- open with O_NOFOLLOW (don't follow symlink)\n"
 " Note1: usually read/write direct IO requests must be blocksize aligned;\n"
 "        some kernels, however, allow sectorsize alignment for direct IO.\n"
 " Note2: the bmap for non-regular files can be obtained provided the file\n"

@@ -203,7 +209,7 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "FLPRTacdfm:nrstx")) != EOF) {
 		switch (c) {
 		case 'F':
 			/* Ignored / deprecated now, handled automatically */

@@ -244,6 +250,12 @@ open_f(
 		case 'T':
 			flags |= IO_TMPFILE;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
+		case 'L':
+			flags |= IO_NOFOLLOW;
+			break;
 		default:
 			return command_usage(&open_cmd);
 		}

@@ -257,6 +269,12 @@ open_f(
 		return -1;
 	}
 
+	if ((flags & (IO_PATH|IO_NOFOLLOW)) &&
+	    (flags & ~(IO_PATH|IO_NOFOLLOW))) {
+		fprintf(stderr, _("-P and -L are incompatible with the other options\n"));
+		return -1;
+	}
+
 	fd = openfile(argv[optind], &geometry, flags, mode);
 	if (fd < 0)
 		return 0;

@@ -772,7 +790,7 @@ open_init(void)
 	open_cmd.argmax = -1;
 	open_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
 			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
-	open_cmd.args = _("[-acdrstxT] [-m mode] [path]");
+	open_cmd.args = _("[-acdrstxRTPL] [-m mode] [path]");
 	open_cmd.oneline = _("open the file specified by path");
 	open_cmd.help = open_help;
man/man8/xfs_io.8 +11 -2
index 022f0ea..e77be40 100644
@@ -122,7 +122,7 @@ command for more details on any command.
 Display a list of all open files and (optionally) switch to an alternate
 current open file.
 .TP
-.BI "open [[ \-acdfrstRT ] " path " ]"
+.BI "open [[ \-acdfrstRTPL ] " path " ]"
 Closes the current file, and opens the file specified by
 .I path
 instead. Without any arguments, displays statistics about the current

@@ -164,6 +164,16 @@ option.
 .B \-R
 marks the file as a realtime XFS file after
 opening it, if it is not already marked as such.
+.TP
+.B \-P
+opens the path as a referent only (O_PATH).  This is incompatible with other
+flags specifying other O_xxx flags apart from
+.BR \-L .
+.TP
+.B \-L
+doesn't follow symlinks (O_NOFOLLOW).  This is incompatible with other
+flags specifying other O_xxx flags apart from
+.BR \-P .
 .PD
 .RE
 .TP
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
In reply to David Howells
Quoted 6 lines · David Howells

xfs_io: Allow "open -P" to specify O_PATH

Allow "open -P" to specify O_PATH so that paths which would otherwise be unopenable might be opened for stat()'ing. Such things include files that would incur an access error or device files for which no corresponding driver is available.

Given all the trouble you ran into with this (and I guess I'm not too surprised), it might make more sense to change stat_f() and statx_f() to be able to take a pathname separate from the target pathname on the xfs_io commandline, i.e. something like:

xfs_io -c "stat foo"

instead of

xfs_io -P -c "stat" foo

-Eric

Quoted 100 lines
Signed-off-by David Howells <[email protected]>
---
 io/io.h           |    1 +
 io/open.c         |   13 ++++++++++++-
 man/man8/xfs_io.8 |    6 +++++-
 3 files changed, 18 insertions(+), 2 deletions(-)
io/io.h +1 -0
index 952bdb8..b49a133 100644
@@ -38,6 +38,7 @@
 #define IO_FOREIGN	(1<<7)
 #define IO_NONBLOCK	(1<<8)
 #define IO_TMPFILE	(1<<9)
+#define IO_PATH		(1<<10)
 
 /*
  * Regular file I/O control
io/open.c +12 -1
index 2ed55cf..9eba8ae 100644
@@ -72,6 +72,8 @@ openfile(
 		oflags |= O_NONBLOCK;
 	if (flags & IO_TMPFILE)
 		oflags |= O_TMPFILE;
+	if (flags & IO_PATH)
+		oflags |= O_PATH;
 
 	fd = open(path, oflags, mode);
 	if (fd < 0) {

@@ -179,6 +181,7 @@ open_help(void)
 " -t -- open with O_TRUNC (truncate the file to zero length if it exists)\n"
 " -R -- mark the file as a realtime XFS file immediately after opening it\n"
 " -T -- open with O_TMPFILE (create a file not visible in the namespace)\n"
+" -P -- open with O_PATH (create an fd that is merely a location reference)\n"
 " Note1: usually read/write direct IO requests must be blocksize aligned;\n"
 "        some kernels, however, allow sectorsize alignment for direct IO.\n"
 " Note2: the bmap for non-regular files can be obtained provided the file\n"

@@ -203,7 +206,7 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "FPRTacdfm:nrstx")) != EOF) {
 		switch (c) {
 		case 'F':
 			/* Ignored / deprecated now, handled automatically */

@@ -244,6 +247,9 @@ open_f(
 		case 'T':
 			flags |= IO_TMPFILE;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
 		default:
 			return command_usage(&open_cmd);
 		}

@@ -257,6 +263,11 @@ open_f(
 		return -1;
 	}
 
+	if ((flags & IO_PATH) && (flags & ~IO_PATH)) {
+		fprintf(stderr, _("-P is incompatible with other options\n"));
+		return -1;
+	}
+
 	fd = openfile(argv[optind], &geometry, flags, mode);
 	if (fd < 0)
 		return 0;
man/man8/xfs_io.8 +5 -2
index 022f0ea..77ba760 100644
@@ -122,7 +122,7 @@ command for more details on any command.
 Display a list of all open files and (optionally) switch to an alternate
 current open file.
 .TP
-.BI "open [[ \-acdfrstRT ] " path " ]"
+.BI "open [[ \-acdfrstRTP ] " path " ]"
 Closes the current file, and opens the file specified by
 .I path
 instead. Without any arguments, displays statistics about the current

@@ -164,6 +164,10 @@ option.
 .B \-R
 marks the file as a realtime XFS file after
 opening it, if it is not already marked as such.
+.TP
+.B \-P
+opens the path as a referent only (O_PATH).  This is incompatible with other
+flags specifying other O_xxx flags.
 .PD
 .RE
 .TP
--

To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html