* [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h
@ 2021-02-04 11:03 Martin Doucha
2021-02-04 11:03 ` [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library Martin Doucha
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Martin Doucha @ 2021-02-04 11:03 UTC (permalink / raw)
To: ltp
Fallback io_uring syscall wrappers were not defined as static inline. This
will lead to linker issues when safe function variants get added to LTP
library.
Also add check for <linux/io_uring.h> to configure script and #include it
in the LAPI header if it's available.
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
Changes since v1:
- None (changes only in patch 4)
configure.ac | 1 +
include/lapi/io_uring.h | 17 +++++++++++------
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/configure.ac b/configure.ac
index 8bdb96300..9fb50c173 100644
--- a/configure.ac
+++ b/configure.ac
@@ -49,6 +49,7 @@ AC_CHECK_HEADERS_ONCE([ \
linux/if_alg.h \
linux/if_ether.h \
linux/if_packet.h \
+ linux/io_uring.h \
linux/keyctl.h \
linux/mempolicy.h \
linux/module.h \
diff --git a/include/lapi/io_uring.h b/include/lapi/io_uring.h
index 174e81e4d..9d280f155 100644
--- a/include/lapi/io_uring.h
+++ b/include/lapi/io_uring.h
@@ -18,6 +18,10 @@
#include "lapi/syscalls.h"
+#ifdef HAVE_LINUX_IO_URING_H
+#include <linux/io_uring.h>
+#endif
+
#ifndef IOSQE_FIXED_FILE
#ifndef __kernel_rwf_t
@@ -260,8 +264,8 @@ struct io_uring_probe {
#ifndef HAVE_IO_URING_REGISTER
-int io_uring_register(int fd, unsigned int opcode, void *arg,
- unsigned int nr_args)
+static inline int io_uring_register(int fd, unsigned int opcode, void *arg,
+ unsigned int nr_args)
{
return tst_syscall(__NR_io_uring_register, fd, opcode, arg, nr_args);
}
@@ -269,22 +273,23 @@ int io_uring_register(int fd, unsigned int opcode, void *arg,
#ifndef HAVE_IO_URING_SETUP
-int io_uring_setup(unsigned int entries, struct io_uring_params *p)
+static inline int io_uring_setup(unsigned int entries,
+ struct io_uring_params *p)
{
return tst_syscall(__NR_io_uring_setup, entries, p);
}
#endif /* HAVE_IO_URING_SETUP */
#ifndef HAVE_IO_URING_ENTER
-int io_uring_enter(int fd, unsigned int to_submit, unsigned int min_complete,
- unsigned int flags, sigset_t *sig)
+static inline int io_uring_enter(int fd, unsigned int to_submit,
+ unsigned int min_complete, unsigned int flags, sigset_t *sig)
{
return tst_syscall(__NR_io_uring_enter, fd, to_submit, min_complete,
flags, sig, _NSIG / 8);
}
#endif /* HAVE_IO_URING_ENTER */
-void io_uring_setup_supported_by_kernel(void)
+static inline void io_uring_setup_supported_by_kernel(void)
{
if ((tst_kvercmp(5, 1, 0)) < 0) {
TEST(syscall(__NR_io_uring_setup, NULL, 0));
--
2.30.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library
2021-02-04 11:03 [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Martin Doucha
@ 2021-02-04 11:03 ` Martin Doucha
2021-02-05 15:56 ` Petr Vorel
2021-02-04 11:03 ` [LTP] [PATCH v2 3/4] Add CAP_SYS_CHROOT to lapi/capability.h Martin Doucha
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2021-02-04 11:03 UTC (permalink / raw)
To: ltp
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
include/tst_safe_io_uring.h | 63 +++++++++++++++++++++
lib/tst_safe_io_uring.c | 108 ++++++++++++++++++++++++++++++++++++
2 files changed, 171 insertions(+)
create mode 100644 include/tst_safe_io_uring.h
create mode 100644 lib/tst_safe_io_uring.c
diff --git a/include/tst_safe_io_uring.h b/include/tst_safe_io_uring.h
new file mode 100644
index 000000000..fa416e35c
--- /dev/null
+++ b/include/tst_safe_io_uring.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) Linux Test Project, 2021
+ */
+
+#ifndef TST_IO_URING_H__
+#define TST_IO_URING_H__
+
+#include "config.h"
+#include "lapi/io_uring.h"
+
+struct tst_io_uring {
+ int fd;
+ void *sqr_base, *cqr_base;
+ /* buffer sizes in bytes for unmapping */
+ size_t sqr_mapsize, cqr_mapsize;
+
+ /* Number of entries in the ring buffers */
+ uint32_t sqr_size, cqr_size;
+
+ /* Submission queue pointers */
+ struct io_uring_sqe *sqr_entries;
+ const uint32_t *sqr_head, *sqr_mask, *sqr_flags, *sqr_dropped;
+ uint32_t *sqr_tail, *sqr_array;
+
+ /* Completion queue pointers */
+ const struct io_uring_cqe *cqr_entries;
+ const uint32_t *cqr_tail, *cqr_mask, *cqr_overflow;
+ uint32_t *cqr_head;
+
+};
+
+/*
+ * Call io_uring_setup() with given arguments and prepare memory mappings
+ * into the tst_io_uring structure passed in the third argument.
+ */
+#define SAFE_IO_URING_INIT(entries, params, uring) \
+ safe_io_uring_init(__FILE__, __LINE__, (entries), (params), (uring))
+int safe_io_uring_init(const char *file, const int lineno,
+ unsigned int entries, struct io_uring_params *params,
+ struct tst_io_uring *uring);
+
+/*
+ * Release io_uring mappings and close the file descriptor. uring->fd will
+ * be set to -1 after close.
+ */
+#define SAFE_IO_URING_CLOSE(uring) \
+ safe_io_uring_close(__FILE__, __LINE__, (uring))
+int safe_io_uring_close(const char *file, const int lineno,
+ struct tst_io_uring *uring);
+
+/*
+ * Call io_uring_enter() and check for errors. The "strict" argument controls
+ * pedantic check whether return value is equal to "to_submit" argument.
+ */
+#define SAFE_IO_URING_ENTER(strict, fd, to_submit, min_complete, flags, sig) \
+ safe_io_uring_enter(__FILE__, __LINE__, (strict), (fd), (to_submit), \
+ (min_complete), (flags), (sig))
+int safe_io_uring_enter(const char *file, const int lineno, int strict,
+ int fd, unsigned int to_submit, unsigned int min_complete,
+ unsigned int flags, sigset_t *sig);
+
+#endif /* TST_IO_URING_H__ */
diff --git a/lib/tst_safe_io_uring.c b/lib/tst_safe_io_uring.c
new file mode 100644
index 000000000..f300fd38c
--- /dev/null
+++ b/lib/tst_safe_io_uring.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 SUSE LLC <mdoucha@suse.cz>
+ */
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "tst_safe_io_uring.h"
+
+int safe_io_uring_init(const char *file, const int lineno,
+ unsigned int entries, struct io_uring_params *params,
+ struct tst_io_uring *uring)
+{
+ errno = 0;
+ uring->fd = io_uring_setup(entries, params);
+
+ if (uring->fd == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "io_uring_setup() failed");
+ return uring->fd;
+ } else if (uring->fd < 0) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "io_uring_setup() returned invalid value %d",
+ uring->fd);
+ return uring->fd;
+ }
+
+ uring->sqr_size = params->sq_entries;
+ uring->cqr_size = params->cq_entries;
+ uring->sqr_mapsize = params->sq_off.array +
+ params->sq_entries * sizeof(__u32);
+ uring->cqr_mapsize = params->cq_off.cqes +
+ params->cq_entries * sizeof(struct io_uring_cqe);
+
+ uring->sqr_base = safe_mmap(file, lineno, NULL, uring->sqr_mapsize,
+ PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
+ IORING_OFF_SQ_RING);
+
+ if (uring->sqr_base == MAP_FAILED)
+ return -1;
+
+ uring->sqr_entries = safe_mmap(file, lineno, NULL,
+ params->sq_entries * sizeof(struct io_uring_sqe),
+ PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
+ IORING_OFF_SQES);
+
+ if (uring->sqr_entries == MAP_FAILED)
+ return -1;
+
+ uring->cqr_base = safe_mmap(file, lineno, NULL, uring->cqr_mapsize,
+ PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
+ IORING_OFF_CQ_RING);
+
+ if (uring->cqr_base == MAP_FAILED)
+ return -1;
+
+ uring->sqr_head = uring->sqr_base + params->sq_off.head;
+ uring->sqr_tail = uring->sqr_base + params->sq_off.tail;
+ uring->sqr_mask = uring->sqr_base + params->sq_off.ring_mask;
+ uring->sqr_flags = uring->sqr_base + params->sq_off.flags;
+ uring->sqr_dropped = uring->sqr_base + params->sq_off.dropped;
+ uring->sqr_array = uring->sqr_base + params->sq_off.array;
+
+ uring->cqr_head = uring->cqr_base + params->cq_off.head;
+ uring->cqr_tail = uring->cqr_base + params->cq_off.tail;
+ uring->cqr_mask = uring->cqr_base + params->cq_off.ring_mask;
+ uring->cqr_overflow = uring->cqr_base + params->cq_off.overflow;
+ uring->cqr_entries = uring->cqr_base + params->cq_off.cqes;
+ return uring->fd;
+}
+
+int safe_io_uring_close(const char *file, const int lineno,
+ struct tst_io_uring *uring)
+{
+ int ret;
+
+ safe_munmap(file, lineno, NULL, uring->cqr_base, uring->cqr_mapsize);
+ safe_munmap(file, lineno, NULL, uring->sqr_entries,
+ uring->sqr_size * sizeof(struct io_uring_sqe));
+ safe_munmap(file, lineno, NULL, uring->sqr_base, uring->sqr_mapsize);
+ ret = safe_close(file, lineno, NULL, uring->fd);
+ uring->fd = -1;
+ return ret;
+}
+
+int safe_io_uring_enter(const char *file, const int lineno, int strict,
+ int fd, unsigned int to_submit, unsigned int min_complete,
+ unsigned int flags, sigset_t *sig)
+{
+ int ret;
+
+ errno = 0;
+ ret = io_uring_enter(fd, to_submit, min_complete, flags, sig);
+
+ if (ret == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "io_uring_enter() failed");
+ } else if (ret < 0) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid io_uring_enter() return value %d", ret);
+ } else if (strict && to_submit != (unsigned int)ret) {
+ tst_brk_(file, lineno, TBROK,
+ "io_uring_enter() submitted %d items (expected %d)",
+ ret, to_submit);
+ }
+
+ return ret;
+}
--
2.30.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 3/4] Add CAP_SYS_CHROOT to lapi/capability.h
2021-02-04 11:03 [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Martin Doucha
2021-02-04 11:03 ` [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library Martin Doucha
@ 2021-02-04 11:03 ` Martin Doucha
2021-02-04 11:03 ` [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373 Martin Doucha
2021-02-05 16:09 ` [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Petr Vorel
3 siblings, 0 replies; 11+ messages in thread
From: Martin Doucha @ 2021-02-04 11:03 UTC (permalink / raw)
To: ltp
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
include/lapi/capability.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/lapi/capability.h b/include/lapi/capability.h
index fde27efd3..95cb6819b 100644
--- a/include/lapi/capability.h
+++ b/include/lapi/capability.h
@@ -24,6 +24,10 @@
# define CAP_NET_RAW 13
#endif
+#ifndef CAP_SYS_CHROOT
+# define CAP_SYS_CHROOT 18
+#endif
+
#ifndef CAP_SYS_ADMIN
# define CAP_SYS_ADMIN 21
#endif
--
2.30.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373
2021-02-04 11:03 [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Martin Doucha
2021-02-04 11:03 ` [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library Martin Doucha
2021-02-04 11:03 ` [LTP] [PATCH v2 3/4] Add CAP_SYS_CHROOT to lapi/capability.h Martin Doucha
@ 2021-02-04 11:03 ` Martin Doucha
2021-02-05 16:49 ` Petr Vorel
2021-02-05 16:09 ` [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Petr Vorel
3 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2021-02-04 11:03 UTC (permalink / raw)
To: ltp
Fixes #770
Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
Changes since v1:
- Added another patch reference
runtest/cve | 1 +
runtest/syscalls | 1 +
.../kernel/syscalls/io_uring/io_uring02.c | 204 ++++++++++++++++++
3 files changed, 206 insertions(+)
create mode 100644 testcases/kernel/syscalls/io_uring/io_uring02.c
diff --git a/runtest/cve b/runtest/cve
index 0bb1983bc..f650854f9 100644
--- a/runtest/cve
+++ b/runtest/cve
@@ -60,3 +60,4 @@ cve-2019-8912 af_alg07
cve-2020-11494 pty04
cve-2020-14386 sendto03
cve-2020-14416 pty03
+cve-2020-29373 io_uring02
diff --git a/runtest/syscalls b/runtest/syscalls
index 11a1e83c2..dbb33a2cd 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1712,3 +1712,4 @@ statx07 statx07
membarrier01 membarrier01
io_uring01 io_uring01
+io_uring02 io_uring02
diff --git a/testcases/kernel/syscalls/io_uring/io_uring02.c b/testcases/kernel/syscalls/io_uring/io_uring02.c
new file mode 100644
index 000000000..08f4a1714
--- /dev/null
+++ b/testcases/kernel/syscalls/io_uring/io_uring02.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2021 SUSE LLC
+ * Author: Nicolai Stange <nstange@suse.de>
+ * LTP port: Martin Doucha <mdoucha@suse.cz>
+ *
+ * CVE-2020-29373
+ *
+ * Check that io_uring does not bypass chroot. Fixed in:
+ *
+ * commit 9392a27d88b9707145d713654eb26f0c29789e50
+ * Author: Jens Axboe <axboe@kernel.dk>
+ * Date: Thu Feb 6 21:42:51 2020 -0700
+ *
+ * io-wq: add support for inheriting ->fs
+ *
+ * commit ff002b30181d30cdfbca316dadd099c3ca0d739c
+ * Author: Jens Axboe <axboe@kernel.dk>
+ * Date: Fri Feb 7 16:05:21 2020 -0700
+ *
+ * io_uring: grab ->fs as part of async preparation
+ */
+
+#include <stdio.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include "tst_test.h"
+#include "tst_safe_io_uring.h"
+
+#define CHROOT_DIR "test_root"
+#define SOCK_NAME "sock"
+#define SPAM_MARK 0xfa7
+#define BEEF_MARK 0xbeef
+
+static struct sockaddr_un addr;
+static int sendsock = -1, recvsock = -1, sockpair[2] = {-1, -1};
+static struct io_uring_params params;
+static struct tst_io_uring uring = { .fd = -1 };
+static char buf[16];
+static struct iovec iov = {
+ .iov_base = buf,
+ .iov_len = sizeof(buf)
+};
+
+static struct msghdr spam_header = {
+ .msg_name = NULL,
+ .msg_namelen = 0,
+ .msg_iov = &iov,
+ .msg_iovlen = 1
+};
+
+static struct msghdr beef_header = {
+ .msg_name = &addr,
+ .msg_namelen = sizeof(addr),
+ .msg_iov = &iov,
+ .msg_iovlen = 1
+};
+
+static void setup(void)
+{
+ char *tmpdir = tst_get_tmpdir();
+ int ret;
+
+ addr.sun_family = AF_UNIX;
+ ret = snprintf(addr.sun_path, sizeof(addr.sun_path), "%s/%s", tmpdir,
+ SOCK_NAME);
+ free(tmpdir);
+
+ if (ret >= (int)sizeof(addr.sun_path))
+ tst_brk(TBROK, "Tempdir path is too long");
+
+ io_uring_setup_supported_by_kernel();
+
+ sendsock = SAFE_SOCKET(AF_UNIX, SOCK_DGRAM, 0);
+ recvsock = SAFE_SOCKET(AF_UNIX, SOCK_DGRAM, 0);
+ SAFE_BIND(recvsock, (struct sockaddr *)&addr, sizeof(addr));
+
+ SAFE_MKDIR(CHROOT_DIR, 0755);
+ SAFE_CHROOT(CHROOT_DIR);
+}
+
+static void run(void)
+{
+ uint32_t i, count, tail;
+ int beef_found = 0;
+ struct io_uring_sqe *sqe_ptr;
+ const struct io_uring_cqe *cqe_ptr;
+
+ SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sockpair);
+ SAFE_SETSOCKOPT_INT(sockpair[0], SOL_SOCKET, SO_SNDBUF,
+ 32+sizeof(buf));
+ SAFE_FCNTL(sockpair[0], F_SETFL, O_NONBLOCK);
+
+ SAFE_IO_URING_INIT(512, ¶ms, &uring);
+ sqe_ptr = uring.sqr_entries;
+
+ /* Add spam requests to force async processing of the real test */
+ for (i = 0, tail = *uring.sqr_tail; i < 255; i++, tail++, sqe_ptr++) {
+ memset(sqe_ptr, 0, sizeof(*sqe_ptr));
+ sqe_ptr->opcode = IORING_OP_SENDMSG;
+ sqe_ptr->flags = IOSQE_IO_DRAIN;
+ sqe_ptr->fd = sockpair[0];
+ sqe_ptr->addr = (__u64)&spam_header;
+ sqe_ptr->user_data = SPAM_MARK;
+ uring.sqr_array[tail & *uring.sqr_mask] = i;
+ }
+
+ /* Add the real test to queue */
+ memset(sqe_ptr, 0, sizeof(*sqe_ptr));
+ sqe_ptr->opcode = IORING_OP_SENDMSG;
+ sqe_ptr->flags = IOSQE_IO_DRAIN;
+ sqe_ptr->fd = sendsock;
+ sqe_ptr->addr = (__u64)&beef_header;
+ sqe_ptr->user_data = BEEF_MARK;
+ uring.sqr_array[tail & *uring.sqr_mask] = i;
+ count = ++i;
+ tail++;
+
+ __atomic_store(uring.sqr_tail, &tail, __ATOMIC_RELEASE);
+ SAFE_IO_URING_ENTER(1, uring.fd, count, count, IORING_ENTER_GETEVENTS,
+ NULL);
+
+ /* Check test results */
+ __atomic_load(uring.cqr_tail, &tail, __ATOMIC_ACQUIRE);
+
+ for (i = *uring.cqr_head; i != tail; i++, count--) {
+ cqe_ptr = uring.cqr_entries + (i & *uring.cqr_mask);
+ TST_ERR = -cqe_ptr->res;
+
+ if (cqe_ptr->user_data == SPAM_MARK) {
+ if (cqe_ptr->res >= 0 || cqe_ptr->res == -EAGAIN)
+ continue;
+
+ tst_res(TFAIL | TTERRNO,
+ "Spam request failed unexpectedly");
+ continue;
+ }
+
+ if (cqe_ptr->user_data != BEEF_MARK) {
+ tst_res(TFAIL, "Unexpected entry in completion queue");
+ count++;
+ continue;
+ }
+
+ beef_found = 1;
+
+ if (cqe_ptr->res >= 0) {
+ tst_res(TFAIL, "Write outside chroot succeeded.");
+ } else if (cqe_ptr->res != -ENOENT) {
+ tst_res(TFAIL | TTERRNO,
+ "Write outside chroot failed unexpectedly");
+ } else {
+ tst_res(TPASS,
+ "Write outside chroot failed as expected");
+ }
+ }
+
+ __atomic_store(uring.cqr_head, &i, __ATOMIC_RELEASE);
+
+ if (!beef_found)
+ tst_res(TFAIL, "Write outside chroot result not found");
+
+ if (count)
+ tst_res(TFAIL, "Wrong number of entries in completion queue");
+
+ /* iteration cleanup */
+ SAFE_IO_URING_CLOSE(&uring);
+ SAFE_CLOSE(sockpair[0]);
+ SAFE_CLOSE(sockpair[1]);
+}
+
+static void cleanup(void)
+{
+ if (uring.fd >= 0)
+ SAFE_IO_URING_CLOSE(&uring);
+
+ if (sockpair[0] >= 0) {
+ SAFE_CLOSE(sockpair[0]);
+ SAFE_CLOSE(sockpair[1]);
+ }
+
+ if (recvsock >= 0)
+ SAFE_CLOSE(recvsock);
+
+ if (sendsock >= 0)
+ SAFE_CLOSE(sendsock);
+}
+
+static struct tst_test test = {
+ .test_all = run,
+ .setup = setup,
+ .cleanup = cleanup,
+ .needs_tmpdir = 1,
+ .caps = (struct tst_cap []) {
+ TST_CAP(TST_CAP_REQ, CAP_SYS_CHROOT),
+ {}
+ },
+ .tags = (const struct tst_tag[]) {
+ {"linux-git", "9392a27d88b9"},
+ {"linux-git", "ff002b30181d"},
+ {"CVE", "2020-29373"},
+ {}
+ }
+};
--
2.30.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library
2021-02-04 11:03 ` [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library Martin Doucha
@ 2021-02-05 15:56 ` Petr Vorel
2021-02-05 16:03 ` Martin Doucha
0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2021-02-05 15:56 UTC (permalink / raw)
To: ltp
Hi Martin,
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> +++ b/include/tst_safe_io_uring.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) Linux Test Project, 2021
> + */
> +
> +#ifndef TST_IO_URING_H__
> +#define TST_IO_URING_H__
> +
> +#include "config.h"
> +#include "lapi/io_uring.h"
> +
> +struct tst_io_uring {
> + int fd;
> + void *sqr_base, *cqr_base;
> + /* buffer sizes in bytes for unmapping */
> + size_t sqr_mapsize, cqr_mapsize;
> +
> + /* Number of entries in the ring buffers */
> + uint32_t sqr_size, cqr_size;
> +
> + /* Submission queue pointers */
> + struct io_uring_sqe *sqr_entries;
> + const uint32_t *sqr_head, *sqr_mask, *sqr_flags, *sqr_dropped;
> + uint32_t *sqr_tail, *sqr_array;
> +
> + /* Completion queue pointers */
> + const struct io_uring_cqe *cqr_entries;
> + const uint32_t *cqr_tail, *cqr_mask, *cqr_overflow;
> + uint32_t *cqr_head;
> +
nit: blank line.
> +};
...
> +++ b/lib/tst_safe_io_uring.c
...
> + uring->sqr_base = safe_mmap(file, lineno, NULL, uring->sqr_mapsize,
> + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
> + IORING_OFF_SQ_RING);
> +
> + if (uring->sqr_base == MAP_FAILED)
> + return -1;
IMHO this is not needed, safe_mmap() breaks on rval == MAP_FAILED.
> +
> + uring->sqr_entries = safe_mmap(file, lineno, NULL,
> + params->sq_entries * sizeof(struct io_uring_sqe),
> + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
> + IORING_OFF_SQES);
> +
> + if (uring->sqr_entries == MAP_FAILED)
> + return -1;
And here.
> +
> + uring->cqr_base = safe_mmap(file, lineno, NULL, uring->cqr_mapsize,
> + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
> + IORING_OFF_CQ_RING);
> +
> + if (uring->cqr_base == MAP_FAILED)
> + return -1;
And here.
No need to repost, these two can be removed before merge.
The rest LGTM.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library
2021-02-05 15:56 ` Petr Vorel
@ 2021-02-05 16:03 ` Martin Doucha
2021-02-05 17:06 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2021-02-05 16:03 UTC (permalink / raw)
To: ltp
On 05. 02. 21 16:56, Petr Vorel wrote:
> Hi Martin,
>
>> +++ b/lib/tst_safe_io_uring.c
> ...
>> + uring->sqr_base = safe_mmap(file, lineno, NULL, uring->sqr_mapsize,
>> + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
>> + IORING_OFF_SQ_RING);
>> +
>> + if (uring->sqr_base == MAP_FAILED)
>> + return -1;
> IMHO this is not needed, safe_mmap() breaks on rval == MAP_FAILED.
Except when called in cleanup() where it prints a warning and returns as
usual. That's why safe_io_uring_init() returns int instead of void in
the first place.
I don't expect that safe_io_uring_init() will be used in cleanup() very
often but I've made all the other SAFE_*() functions cleanup()-safe and
I want to stick with that here as well.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h
2021-02-04 11:03 [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Martin Doucha
` (2 preceding siblings ...)
2021-02-04 11:03 ` [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373 Martin Doucha
@ 2021-02-05 16:09 ` Petr Vorel
3 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-02-05 16:09 UTC (permalink / raw)
To: ltp
Hi Martin,
> Fallback io_uring syscall wrappers were not defined as static inline. This
> will lead to linker issues when safe function variants get added to LTP
> library.
> Also add check for <linux/io_uring.h> to configure script and #include it
> in the LAPI header if it's available.
Thanks for cleanup!
Reviewed-by: Petr Vorel <pvorel@suse.cz>
...
> +++ b/configure.ac
> @@ -49,6 +49,7 @@ AC_CHECK_HEADERS_ONCE([ \
> linux/if_alg.h \
> linux/if_ether.h \
> linux/if_packet.h \
> + linux/io_uring.h \
> linux/keyctl.h \
> linux/mempolicy.h \
> linux/module.h \
...
> +++ b/include/lapi/io_uring.h
...
> +#ifdef HAVE_LINUX_IO_URING_H
> +#include <linux/io_uring.h>
> +#endif
> +
> #ifndef IOSQE_FIXED_FILE
This could be also guarded as ! HAVE_LINUX_IO_URING_H (#else branch), but
keeping IOSQE_FIXED_FILE is probably safer.
> #ifndef __kernel_rwf_t
> @@ -260,8 +264,8 @@ struct io_uring_probe {
> #ifndef HAVE_IO_URING_REGISTER
> -int io_uring_register(int fd, unsigned int opcode, void *arg,
> - unsigned int nr_args)
> +static inline int io_uring_register(int fd, unsigned int opcode, void *arg,
> + unsigned int nr_args)
> {
> return tst_syscall(__NR_io_uring_register, fd, opcode, arg, nr_args);
> }
> @@ -269,22 +273,23 @@ int io_uring_register(int fd, unsigned int opcode, void *arg,
...
Kind regards,
Petr
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373
2021-02-04 11:03 ` [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373 Martin Doucha
@ 2021-02-05 16:49 ` Petr Vorel
2021-02-08 9:37 ` Martin Doucha
0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2021-02-05 16:49 UTC (permalink / raw)
To: ltp
Hi Martin,
> Fixes #770
Nice port thanks!
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Few questions below.
...
> +static void run(void)
> +{
> + uint32_t i, count, tail;
> + int beef_found = 0;
> + struct io_uring_sqe *sqe_ptr;
> + const struct io_uring_cqe *cqe_ptr;
> +
> + SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sockpair);
> + SAFE_SETSOCKOPT_INT(sockpair[0], SOL_SOCKET, SO_SNDBUF,
> + 32+sizeof(buf));
> + SAFE_FCNTL(sockpair[0], F_SETFL, O_NONBLOCK);
> +
> + SAFE_IO_URING_INIT(512, ¶ms, &uring);
> + sqe_ptr = uring.sqr_entries;
> +
> + /* Add spam requests to force async processing of the real test */
> + for (i = 0, tail = *uring.sqr_tail; i < 255; i++, tail++, sqe_ptr++) {
> + memset(sqe_ptr, 0, sizeof(*sqe_ptr));
> + sqe_ptr->opcode = IORING_OP_SENDMSG;
> + sqe_ptr->flags = IOSQE_IO_DRAIN;
> + sqe_ptr->fd = sockpair[0];
> + sqe_ptr->addr = (__u64)&spam_header;
> + sqe_ptr->user_data = SPAM_MARK;
Interesting, original reproducer uses here i
> + uring.sqr_array[tail & *uring.sqr_mask] = i;
> + }
> +
> + /* Add the real test to queue */
> + memset(sqe_ptr, 0, sizeof(*sqe_ptr));
> + sqe_ptr->opcode = IORING_OP_SENDMSG;
> + sqe_ptr->flags = IOSQE_IO_DRAIN;
> + sqe_ptr->fd = sendsock;
> + sqe_ptr->addr = (__u64)&beef_header;
> + sqe_ptr->user_data = BEEF_MARK;
and here also 255, you use much higher 0xbeef.
You probably have a good reason to use here 0xfa7 (higher value). But maybe
explaining why?
> + uring.sqr_array[tail & *uring.sqr_mask] = i;
> + count = ++i;
> + tail++;
> +
> + __atomic_store(uring.sqr_tail, &tail, __ATOMIC_RELEASE);
> + SAFE_IO_URING_ENTER(1, uring.fd, count, count, IORING_ENTER_GETEVENTS,
> + NULL);
> +
> + /* Check test results */
> + __atomic_load(uring.cqr_tail, &tail, __ATOMIC_ACQUIRE);
> +
> + for (i = *uring.cqr_head; i != tail; i++, count--) {
> + cqe_ptr = uring.cqr_entries + (i & *uring.cqr_mask);
> + TST_ERR = -cqe_ptr->res;
> +
> + if (cqe_ptr->user_data == SPAM_MARK) {
> + if (cqe_ptr->res >= 0 || cqe_ptr->res == -EAGAIN)
> + continue;
> +
> + tst_res(TFAIL | TTERRNO,
> + "Spam request failed unexpectedly");
I'm sorry, I'm lost to which TEST*() call this TTERRNO refers (there are mostly
SAFE_*() macros.
> + continue;
> + }
> +
> + if (cqe_ptr->user_data != BEEF_MARK) {
> + tst_res(TFAIL, "Unexpected entry in completion queue");
> + count++;
> + continue;
> + }
> +
> + beef_found = 1;
> +
> + if (cqe_ptr->res >= 0) {
> + tst_res(TFAIL, "Write outside chroot succeeded.");
> + } else if (cqe_ptr->res != -ENOENT) {
> + tst_res(TFAIL | TTERRNO,
And here.
> + "Write outside chroot failed unexpectedly");
> + } else {
> + tst_res(TPASS,
> + "Write outside chroot failed as expected");
> + }
> + }
> +
> + __atomic_store(uring.cqr_head, &i, __ATOMIC_RELEASE);
> +
> + if (!beef_found)
> + tst_res(TFAIL, "Write outside chroot result not found");
> +
> + if (count)
> + tst_res(TFAIL, "Wrong number of entries in completion queue");
> +
> + /* iteration cleanup */
> + SAFE_IO_URING_CLOSE(&uring);
> + SAFE_CLOSE(sockpair[0]);
> + SAFE_CLOSE(sockpair[1]);
> +}
Kind regards,
Petr
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library
2021-02-05 16:03 ` Martin Doucha
@ 2021-02-05 17:06 ` Petr Vorel
0 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-02-05 17:06 UTC (permalink / raw)
To: ltp
Hi Martin,
> >> +++ b/lib/tst_safe_io_uring.c
> > ...
> >> + uring->sqr_base = safe_mmap(file, lineno, NULL, uring->sqr_mapsize,
> >> + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE, uring->fd,
> >> + IORING_OFF_SQ_RING);
> >> +
> >> + if (uring->sqr_base == MAP_FAILED)
> >> + return -1;
> > IMHO this is not needed, safe_mmap() breaks on rval == MAP_FAILED.
> Except when called in cleanup() where it prints a warning and returns as
> usual. That's why safe_io_uring_init() returns int instead of void in
> the first place.
> I don't expect that safe_io_uring_init() will be used in cleanup() very
> often but I've made all the other SAFE_*() functions cleanup()-safe and
> I want to stick with that here as well.
Good point, makes sense. Thanks for an explanation.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373
2021-02-05 16:49 ` Petr Vorel
@ 2021-02-08 9:37 ` Martin Doucha
2021-02-08 9:48 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Martin Doucha @ 2021-02-08 9:37 UTC (permalink / raw)
To: ltp
On 05. 02. 21 17:49, Petr Vorel wrote:
> Hi Martin,
>
>> Fixes #770
>
> Nice port thanks!
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Few questions below.
>
> ...
>> +static void run(void)
>> +{
>> + uint32_t i, count, tail;
>> + int beef_found = 0;
>> + struct io_uring_sqe *sqe_ptr;
>> + const struct io_uring_cqe *cqe_ptr;
>> +
>> + SAFE_SOCKETPAIR(AF_UNIX, SOCK_DGRAM, 0, sockpair);
>> + SAFE_SETSOCKOPT_INT(sockpair[0], SOL_SOCKET, SO_SNDBUF,
>> + 32+sizeof(buf));
>> + SAFE_FCNTL(sockpair[0], F_SETFL, O_NONBLOCK);
>> +
>> + SAFE_IO_URING_INIT(512, ¶ms, &uring);
>> + sqe_ptr = uring.sqr_entries;
>> +
>> + /* Add spam requests to force async processing of the real test */
>> + for (i = 0, tail = *uring.sqr_tail; i < 255; i++, tail++, sqe_ptr++) {
>> + memset(sqe_ptr, 0, sizeof(*sqe_ptr));
>> + sqe_ptr->opcode = IORING_OP_SENDMSG;
>> + sqe_ptr->flags = IOSQE_IO_DRAIN;
>> + sqe_ptr->fd = sockpair[0];
>> + sqe_ptr->addr = (__u64)&spam_header;
>> + sqe_ptr->user_data = SPAM_MARK;
> Interesting, original reproducer uses here i
>
>> + uring.sqr_array[tail & *uring.sqr_mask] = i;
>> + }
>> +
>> + /* Add the real test to queue */
>> + memset(sqe_ptr, 0, sizeof(*sqe_ptr));
>> + sqe_ptr->opcode = IORING_OP_SENDMSG;
>> + sqe_ptr->flags = IOSQE_IO_DRAIN;
>> + sqe_ptr->fd = sendsock;
>> + sqe_ptr->addr = (__u64)&beef_header;
>> + sqe_ptr->user_data = BEEF_MARK;
> and here also 255, you use much higher 0xbeef.
>
> You probably have a good reason to use here 0xfa7 (higher value). But maybe
> explaining why?
The good reason is that I like puns. sqe_ptr->user_data is not processed
by the kernel in any way except for copying the value into the
completion queue when the I/O request finishes. And we don't care
whether we can tell apart the spam request results from one another so
giving them all the same marker is good enough.
>> + uring.sqr_array[tail & *uring.sqr_mask] = i;
>> + count = ++i;
>> + tail++;
>> +
>> + __atomic_store(uring.sqr_tail, &tail, __ATOMIC_RELEASE);
>> + SAFE_IO_URING_ENTER(1, uring.fd, count, count, IORING_ENTER_GETEVENTS,
>> + NULL);
>> +
>> + /* Check test results */
>> + __atomic_load(uring.cqr_tail, &tail, __ATOMIC_ACQUIRE);
>> +
>> + for (i = *uring.cqr_head; i != tail; i++, count--) {
>> + cqe_ptr = uring.cqr_entries + (i & *uring.cqr_mask);
>> + TST_ERR = -cqe_ptr->res;
>> +
>> + if (cqe_ptr->user_data == SPAM_MARK) {
>> + if (cqe_ptr->res >= 0 || cqe_ptr->res == -EAGAIN)
>> + continue;
>> +
>> + tst_res(TFAIL | TTERRNO,
>> + "Spam request failed unexpectedly");
> I'm sorry, I'm lost to which TEST*() call this TTERRNO refers (there are mostly
> SAFE_*() macros.
I'm setting TST_ERR manually 6 lines above the tst_res() call. The errno
value is in cqe_ptr->res.
--
Martin Doucha mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic
^ permalink raw reply [flat|nested] 11+ messages in thread
* [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373
2021-02-08 9:37 ` Martin Doucha
@ 2021-02-08 9:48 ` Petr Vorel
0 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2021-02-08 9:48 UTC (permalink / raw)
To: ltp
Hi Martin,
> >> + /* Add spam requests to force async processing of the real test */
> >> + for (i = 0, tail = *uring.sqr_tail; i < 255; i++, tail++, sqe_ptr++) {
> >> + memset(sqe_ptr, 0, sizeof(*sqe_ptr));
> >> + sqe_ptr->opcode = IORING_OP_SENDMSG;
> >> + sqe_ptr->flags = IOSQE_IO_DRAIN;
> >> + sqe_ptr->fd = sockpair[0];
> >> + sqe_ptr->addr = (__u64)&spam_header;
> >> + sqe_ptr->user_data = SPAM_MARK;
> > Interesting, original reproducer uses here i
> >> + uring.sqr_array[tail & *uring.sqr_mask] = i;
> >> + }
> >> +
> >> + /* Add the real test to queue */
> >> + memset(sqe_ptr, 0, sizeof(*sqe_ptr));
> >> + sqe_ptr->opcode = IORING_OP_SENDMSG;
> >> + sqe_ptr->flags = IOSQE_IO_DRAIN;
> >> + sqe_ptr->fd = sendsock;
> >> + sqe_ptr->addr = (__u64)&beef_header;
> >> + sqe_ptr->user_data = BEEF_MARK;
> > and here also 255, you use much higher 0xbeef.
> > You probably have a good reason to use here 0xfa7 (higher value). But maybe
> > explaining why?
> The good reason is that I like puns. sqe_ptr->user_data is not processed
> by the kernel in any way except for copying the value into the
> completion queue when the I/O request finishes. And we don't care
> whether we can tell apart the spam request results from one another so
> giving them all the same marker is good enough.
Thanks for an explanation!
> >> + uring.sqr_array[tail & *uring.sqr_mask] = i;
> >> + count = ++i;
> >> + tail++;
> >> +
> >> + __atomic_store(uring.sqr_tail, &tail, __ATOMIC_RELEASE);
> >> + SAFE_IO_URING_ENTER(1, uring.fd, count, count, IORING_ENTER_GETEVENTS,
> >> + NULL);
> >> +
> >> + /* Check test results */
> >> + __atomic_load(uring.cqr_tail, &tail, __ATOMIC_ACQUIRE);
> >> +
> >> + for (i = *uring.cqr_head; i != tail; i++, count--) {
> >> + cqe_ptr = uring.cqr_entries + (i & *uring.cqr_mask);
> >> + TST_ERR = -cqe_ptr->res;
> >> +
> >> + if (cqe_ptr->user_data == SPAM_MARK) {
> >> + if (cqe_ptr->res >= 0 || cqe_ptr->res == -EAGAIN)
> >> + continue;
> >> +
> >> + tst_res(TFAIL | TTERRNO,
> >> + "Spam request failed unexpectedly");
> > I'm sorry, I'm lost to which TEST*() call this TTERRNO refers (there are mostly
> > SAFE_*() macros.
> I'm setting TST_ERR manually 6 lines above the tst_res() call. The errno
> value is in cqe_ptr->res.
Thank you, I'm blind :).
Anyway, merged. Thanks for your work!
BTW: test fails on my openSUSE kernel 5.11.0-rc6, which should have both kernel
fixes.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-02-08 9:48 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 11:03 [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Martin Doucha
2021-02-04 11:03 ` [LTP] [PATCH v2 2/4] Add safe functions for io_uring to LTP library Martin Doucha
2021-02-05 15:56 ` Petr Vorel
2021-02-05 16:03 ` Martin Doucha
2021-02-05 17:06 ` Petr Vorel
2021-02-04 11:03 ` [LTP] [PATCH v2 3/4] Add CAP_SYS_CHROOT to lapi/capability.h Martin Doucha
2021-02-04 11:03 ` [LTP] [PATCH v2 4/4] Add test for CVE 2020-29373 Martin Doucha
2021-02-05 16:49 ` Petr Vorel
2021-02-08 9:37 ` Martin Doucha
2021-02-08 9:48 ` Petr Vorel
2021-02-05 16:09 ` [LTP] [PATCH v2 1/4] Prevent linker issues in lapi/io_uring.h Petr Vorel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.