* Re: [PATCH 2/2] xfs_io: allow open file permissions to be changed
2018-12-02 20:53 ` [PATCH 2/2] xfs_io: allow open file permissions to be changed Dave Chinner
@ 2018-12-03 10:17 ` Jan Tulak
2018-12-03 20:14 ` Dave Chinner
2018-12-03 16:24 ` Darrick J. Wong
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Jan Tulak @ 2018-12-03 10:17 UTC (permalink / raw)
To: Chinner, Dave; +Cc: linux-xfs
On Sun, Dec 2, 2018 at 9:54 PM Dave Chinner <david@fromorbit.com> wrote:
>
> From: Dave Chinner <dchinner@redhat.com>
>
> 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 <dchinner@redhat.com>
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.
> ---
> io/open.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/io/open.c b/io/open.c
> index b1d9a0fa317c..153d4a836d4c 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -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
>
--
Jan Tulak
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] xfs_io: allow open file permissions to be changed
2018-12-03 10:17 ` Jan Tulak
@ 2018-12-03 20:14 ` Dave Chinner
0 siblings, 0 replies; 16+ messages in thread
From: Dave Chinner @ 2018-12-03 20:14 UTC (permalink / raw)
To: Jan Tulak; +Cc: linux-xfs
On Mon, Dec 03, 2018 at 11:17:05AM +0100, Jan Tulak wrote:
> On Sun, Dec 2, 2018 at 9:54 PM Dave Chinner <david@fromorbit.com> wrote:
> >
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > 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 <dchinner@redhat.com>
>
> 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
I forgot that, thanks. So many other things to deal with.
> 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.
If we need more than "make read-only" or "make read-write" then we
can do something more. But for pretty much all the cases where fine
grained permissions are needed, the chmod command itself should
suffice....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] xfs_io: allow open file permissions to be changed
2018-12-02 20:53 ` [PATCH 2/2] xfs_io: allow open file permissions to be changed Dave Chinner
2018-12-03 10:17 ` Jan Tulak
@ 2018-12-03 16:24 ` Darrick J. Wong
2018-12-05 4:02 ` [PATCH 2/2 V2] " Dave Chinner
2018-12-05 4:04 ` [PATCH 2/2] " Eric Sandeen
3 siblings, 0 replies; 16+ messages in thread
From: Darrick J. Wong @ 2018-12-03 16:24 UTC (permalink / raw)
To: Dave Chinner; +Cc: linux-xfs
On Mon, Dec 03, 2018 at 07:53:43AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> 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 <dchinner@redhat.com>
> ---
> io/open.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/io/open.c b/io/open.c
> index b1d9a0fa317c..153d4a836d4c 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -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");
"permissions"...
Also, there needs to be a manpage update for this, or else xfs/293 fails.
--D
> + 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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/2 V2] xfs_io: allow open file permissions to be changed
2018-12-02 20:53 ` [PATCH 2/2] xfs_io: allow open file permissions to be changed Dave Chinner
2018-12-03 10:17 ` Jan Tulak
2018-12-03 16:24 ` Darrick J. Wong
@ 2018-12-05 4:02 ` Dave Chinner
2018-12-05 4:23 ` Eric Sandeen
2018-12-05 4:04 ` [PATCH 2/2] " Eric Sandeen
3 siblings, 1 reply; 16+ messages in thread
From: Dave Chinner @ 2018-12-05 4:02 UTC (permalink / raw)
To: linux-xfs
From: Dave Chinner <dchinner@redhat.com>
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 <dchinner@redhat.com>
---
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(+)
diff --git a/io/open.c b/io/open.c
index 21c0e054f8d2..2663c38e9681 100644
--- a/io/open.c
+++ b/io/open.c
@@ -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);
}
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index f1099c32de66..68e03b4558b9 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -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),
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2 V2] xfs_io: allow open file permissions to be changed
2018-12-05 4:02 ` [PATCH 2/2 V2] " Dave Chinner
@ 2018-12-05 4:23 ` Eric Sandeen
0 siblings, 0 replies; 16+ messages in thread
From: Eric Sandeen @ 2018-12-05 4:23 UTC (permalink / raw)
To: Dave Chinner, linux-xfs
On 12/4/18 10:02 PM, Dave Chinner wrote:
>
> From: Dave Chinner <dchinner@redhat.com>
>
> 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 <dchinner@redhat.com>
whooops we crossed the streams :D
I'll move the man update to the file command section, but:
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 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(+)
>
> diff --git a/io/open.c b/io/open.c
> index 21c0e054f8d2..2663c38e9681 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -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);
> }
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index f1099c32de66..68e03b4558b9 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -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),
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/2] xfs_io: allow open file permissions to be changed
2018-12-02 20:53 ` [PATCH 2/2] xfs_io: allow open file permissions to be changed Dave Chinner
` (2 preceding siblings ...)
2018-12-05 4:02 ` [PATCH 2/2 V2] " Dave Chinner
@ 2018-12-05 4:04 ` Eric Sandeen
3 siblings, 0 replies; 16+ messages in thread
From: Eric Sandeen @ 2018-12-05 4:04 UTC (permalink / raw)
To: Dave Chinner, linux-xfs
On 12/2/18 2:53 PM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
>
> 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 <dchinner@redhat.com>
I'll add:
chmod -r | -w
Changes the permissions of the open file to read-only (0444) or read/write (0664).
just below open/close in the xfs_io manpage and add my:
[sandeen: Add man page entry]
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
if that's cool with you.
> ---
> io/open.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/io/open.c b/io/open.c
> index b1d9a0fa317c..153d4a836d4c 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -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);
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread