On 2/3/23 09:14, Jens Axboe wrote:
On 2/3/23 7:08?AM, Vincent Fu wrote:
On 2/3/23 07:46, Jens Axboe wrote:
On Feb 3, 2023, at 5:35 AM, Kanchan Joshi <joshi.k@xxxxxxxxxxx> wrote:
?Use O_RDONLY flag when read is requested on char-type files.
Signed-off-by: Kanchan Joshi <joshi.k@xxxxxxxxxxx>
---
Before this patch: unexpected permission-denial for unprivileged-user.
$ ls -l /dev/ng0n1
cr--r--r-- 1 root root 242, 0 Feb 3 16:30 /dev/ng0n1
$ ./fio -iodepth=1 -rw=randread -ioengine=io_uring_cmd -bs=4k -numjobs=1
-size=4k -cmd_type=nvme -filename=/dev/ng0n1 -name=t
t: (g=0): rw=randread, bs=(R) 4096B-4096B, (W) 4096B-4096B, (T)
4096B-4096B, ioengine=io_uring_cmd, iodepth=1
fio-3.33-71-g7d7a
Starting 1 process
fio: pid=131312, err=13/file:filesetup.c:805, func=open(/dev/ng0n1),
error=Permission denied
filesetup.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/filesetup.c b/filesetup.c
index 1d3cc5ad..d77b8ba4 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -768,10 +768,7 @@ open_again:
else
from_hash = file_lookup_open(f, flags);
} else if (td_read(td)) {
- if (f->filetype == FIO_TYPE_CHAR && !read_only)
- flags |= O_RDWR;
- else
- flags |= O_RDONLY;
+ flags |= O_RDONLY;
This will break sg like interfaces, where a read is done by writing the command to the char device.
?
Jens Axboe
Kanchan, does it work if you run fio with the --readonly option?
I think we should just make it work. Conceptually, the patch is obviously
fine, it just happens to break some oddball cases like sg/bsg. But maybe
we just add a fio ioengine flag for that, like FIO_RO_NEEDS_RW_OPEN or
something, where the engine can tell us if it needs a writeable open
even or a read-only workload.
Then generic_file_open() can use O_RDONLY for td_read(), except if the
engine has FIO_RO_NEEDS_RW_OPEN set.
Kanchan, can you try the diff below? It's also available at
https://github.com/vincentkfu/fio/tree/rwopen
Vincent
diff --git a/engines/sg.c b/engines/sg.c
index 24783374..0bb5be4a 100644
--- a/engines/sg.c
+++ b/engines/sg.c
@@ -1428,7 +1428,7 @@ static struct ioengine_ops ioengine = {
.open_file = fio_sgio_open,
.close_file = fio_sgio_close,
.get_file_size = fio_sgio_get_file_size,
- .flags = FIO_SYNCIO | FIO_RAWIO,
+ .flags = FIO_SYNCIO | FIO_RAWIO | FIO_RO_NEEDS_RW_OPEN,
.options = options,
.option_struct_size = sizeof(struct sg_options)
};
diff --git a/filesetup.c b/filesetup.c
index 1d3cc5ad..cb7047c5 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -768,7 +768,7 @@ open_again:
else
from_hash = file_lookup_open(f, flags);
} else if (td_read(td)) {
- if (f->filetype == FIO_TYPE_CHAR && !read_only)
+ if (td_ioengine_flagged(td, FIO_RO_NEEDS_RW_OPEN) && !read_only)
flags |= O_RDWR;
else
flags |= O_RDONLY;
diff --git a/ioengines.h b/ioengines.h
index d43540d0..2cb9743e 100644
--- a/ioengines.h
+++ b/ioengines.h
@@ -89,6 +89,8 @@ enum fio_ioengine_flags {
= 1 << 16, /* async ioengine with commit function that sets
issue_time */
FIO_SKIPPABLE_IOMEM_ALLOC
= 1 << 17, /* skip iomem_alloc & iomem_free if job sets mem/iomem */
+ FIO_RO_NEEDS_RW_OPEN
+ = 1 << 18, /* open files in rw mode even if we have a read job */
};
/*
--
