A few tests currently create temporary files in / to obtain an fd for testing further syscalls. This is suboptimal as these files may be used for other purposes, they are not deleted after the tests are run, and we should generally avoid creating files at the root.
We could create files in /tmp instead, with some naming pattern à la mkstemp(), but it turns out there is an even simpler solution. open() can be passed O_TMPFILE to create an unnamed file under a given directory. The file disappears once the fd is closed, so there can be no leftover. This approach is used to implement tmpfile() in glibc, if available.
On the model of tmpfile(), add a new helper tmpfd() in freestanding.h and make use of it in the tests that need a temporary fd, replacing the hardcoded files. We try to create the unnamed file under /tmp if it exists, as /tmp should be writable by any user, while / may not be writable at all.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com ---
v1..v2: * Replaced open_file() with tmpfd() in the new uaccess tests. * Attempt to create the unnamed file under /tmp first, to avoid requiring write access to /.
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/morello/kselfte...
.../selftests/arm64/morello/freestanding.h | 19 +++++++++++++++++++ tools/testing/selftests/arm64/morello/mmap.c | 4 +--- .../selftests/arm64/morello/read_write.c | 7 ++----- .../testing/selftests/arm64/morello/uaccess.c | 13 +------------ 4 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 8dab905d2b54..0969b518bad2 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -8,6 +8,8 @@ #include <stdint.h> #include <stddef.h> #include <asm/unistd.h> +#include <linux/errno.h> +#include <linux/fcntl.h> #include <linux/posix_types.h> #include <linux/resource.h> #include <linux/signal.h> @@ -229,4 +231,21 @@ static inline int waitpid(pid_t pid, int *wstatus, int options) return syscall(__NR_wait4, pid, wstatus, options, 0); }
+/* + * Creates a new temporary file and returns an fd to it. The file has no name + * (see open(2) regarding O_TMPFILE) and is deleted when the fd is closed. + */ +static inline int tmpfd(void) +{ + int fd; + + /* First try /tmp, fall back to / if it doesn't exist */ + fd = syscall(__NR_openat, 0, "/tmp", O_TMPFILE | O_RDWR, 0666); + if (fd == -ENOENT) + fd = syscall(__NR_openat, 0, "/", O_TMPFILE | O_RDWR, 0666); + + ASSERT_GE(fd, 0); + return fd; +} + #endif diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 62fc14d14d46..618eb221d0ae 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -7,7 +7,6 @@ #include <linux/mman.h> #include <linux/errno.h> #include <linux/fs.h> -#include <linux/fcntl.h> #include <cheriintrin.h> #include "freestanding.h"
@@ -65,13 +64,12 @@ void syscall_mmap2(void) { const char msg[] = "foo"; unsigned int msg_len = sizeof(msg); /* No need for the terminator */ - const char *sample_file = "/limbo.dat"; void *addr; int fd; int retval;
/* create a sample file to map onto with mmap */ - fd = syscall(__NR_openat, 0, sample_file, O_RDWR | O_CREAT, FILE_PERM); + fd = tmpfd();
ASSERT_GE(fd, 0);
diff --git a/tools/testing/selftests/arm64/morello/read_write.c b/tools/testing/selftests/arm64/morello/read_write.c index a68b5efeb5ab..6425c8449baa 100644 --- a/tools/testing/selftests/arm64/morello/read_write.c +++ b/tools/testing/selftests/arm64/morello/read_write.c @@ -7,7 +7,6 @@ #include <linux/fs.h> #include <linux/stat.h> #include <linux/uio.h> -#include <asm/fcntl.h> #include <asm/unistd.h>
#include "freestanding.h" @@ -26,8 +25,6 @@ static const char w_vec_msg2[VEC_MSG_LEN] = " is vector\n"; static char r_vec_msg1[VEC_MSG_LEN]; static char r_vec_msg2[VEC_MSG_LEN];
-static const char file[] = "/check_cap.txt"; - static const struct iovec w_iovec[VEC_MSG_NUM] = { {.iov_base = (char *) w_vec_msg1, .iov_len = VEC_MSG_LEN}, {.iov_base = (char *) w_vec_msg2, .iov_len = VEC_MSG_LEN}, @@ -58,8 +55,8 @@ TEST(test_writev)
TEST(test_open) { - fd = syscall(__NR_openat, 0, file, O_RDWR | O_CREAT, 0666); - ASSERT_LE(1, fd) TH_LOG("open failed"); + /* tmpfd() asserts that the openat syscall succeeds */ + fd = tmpfd(); }
TEST(test_read) diff --git a/tools/testing/selftests/arm64/morello/uaccess.c b/tools/testing/selftests/arm64/morello/uaccess.c index a1667d9b6bfc..7a48a35c67c5 100644 --- a/tools/testing/selftests/arm64/morello/uaccess.c +++ b/tools/testing/selftests/arm64/morello/uaccess.c @@ -5,11 +5,8 @@ #include <stddef.h> #include <stdbool.h>
-#include <asm/fcntl.h> -#include <asm-generic/errno-base.h> #include <cheriintrin.h> #include <linux/errno.h> -#include <linux/fs.h> #include <linux/futex.h> #include <linux/mman.h> #include <linux/signal.h> @@ -22,14 +19,11 @@
#define MSG_LEN 16
-static const char file[] = "/my_file.txt"; - static inline int fsopen(const char *string) { return syscall(__NR_fsopen, string); }
- static inline int futex_wake_op(uint32_t *uaddr, uint32_t *uaddr2, uint32_t val3) { return syscall(__NR_futex, uaddr, FUTEX_WAKE_OP, 1, 1, @@ -48,11 +42,6 @@ static int getsockname(int socket, struct sockaddr *sockaddr, int *socklen) return syscall(__NR_getsockname, socket, sockaddr, socklen); }
-static int open_file(void) -{ - return syscall(__NR_openat, 0, file, O_RDWR | O_CREAT, 0666); -} - static inline int preadv(int fd, struct iovec *iov, int iovcnt) { return syscall(__NR_preadv, fd, iov, iovcnt, 0); @@ -181,7 +170,7 @@ TEST(test_explicit_iov_iter) int iovcnt = sizeof(iov) / sizeof(struct iovec); int fd;
- fd = open_file(); + fd = tmpfd(); ASSERT_NE(fd, -1);
iov[0].iov_base = (void *)write_buf0;
On 01/03/2024 17:10, Kevin Brodsky wrote:
A few tests currently create temporary files in / to obtain an fd for testing further syscalls. This is suboptimal as these files may be used for other purposes, they are not deleted after the tests are run, and we should generally avoid creating files at the root.
We could create files in /tmp instead, with some naming pattern à la mkstemp(), but it turns out there is an even simpler solution. open() can be passed O_TMPFILE to create an unnamed file under a given directory. The file disappears once the fd is closed, so there can be no leftover. This approach is used to implement tmpfile() in glibc, if available.
On the model of tmpfile(), add a new helper tmpfd() in freestanding.h and make use of it in the tests that need a temporary fd, replacing the hardcoded files. We try to create the unnamed file under /tmp if it exists, as /tmp should be writable by any user, while / may not be writable at all.
Signed-off-by: Kevin Brodsky kevin.brodsky@arm.com
v1..v2:
- Replaced open_file() with tmpfd() in the new uaccess tests.
- Attempt to create the unnamed file under /tmp first, to avoid requiring write access to /.
Review branch:
https://git.morello-project.org/kbrodsky-arm/linux/-/commits/morello/kselftests_tmpfile_v2
.../selftests/arm64/morello/freestanding.h | 19 +++++++++++++++++++ tools/testing/selftests/arm64/morello/mmap.c | 4 +--- .../selftests/arm64/morello/read_write.c | 7 ++----- .../testing/selftests/arm64/morello/uaccess.c | 13 +------------ 4 files changed, 23 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/arm64/morello/freestanding.h b/tools/testing/selftests/arm64/morello/freestanding.h index 8dab905d2b54..0969b518bad2 100644 --- a/tools/testing/selftests/arm64/morello/freestanding.h +++ b/tools/testing/selftests/arm64/morello/freestanding.h @@ -8,6 +8,8 @@ #include <stdint.h> #include <stddef.h> #include <asm/unistd.h> +#include <linux/errno.h> +#include <linux/fcntl.h> #include <linux/posix_types.h> #include <linux/resource.h> #include <linux/signal.h> @@ -229,4 +231,21 @@ static inline int waitpid(pid_t pid, int *wstatus, int options) return syscall(__NR_wait4, pid, wstatus, options, 0); } +/*
- Creates a new temporary file and returns an fd to it. The file has no name
- (see open(2) regarding O_TMPFILE) and is deleted when the fd is closed.
- */
+static inline int tmpfd(void) +{
- int fd;
- /* First try /tmp, fall back to / if it doesn't exist */
- fd = syscall(__NR_openat, 0, "/tmp", O_TMPFILE | O_RDWR, 0666);
- if (fd == -ENOENT)
fd = syscall(__NR_openat, 0, "/", O_TMPFILE | O_RDWR, 0666);
- ASSERT_GE(fd, 0);
- return fd;
+}
- #endif
diff --git a/tools/testing/selftests/arm64/morello/mmap.c b/tools/testing/selftests/arm64/morello/mmap.c index 62fc14d14d46..618eb221d0ae 100644 --- a/tools/testing/selftests/arm64/morello/mmap.c +++ b/tools/testing/selftests/arm64/morello/mmap.c @@ -7,7 +7,6 @@ #include <linux/mman.h> #include <linux/errno.h> #include <linux/fs.h> -#include <linux/fcntl.h> #include <cheriintrin.h> #include "freestanding.h" @@ -65,13 +64,12 @@ void syscall_mmap2(void) { const char msg[] = "foo"; unsigned int msg_len = sizeof(msg); /* No need for the terminator */
- const char *sample_file = "/limbo.dat"; void *addr; int fd; int retval;
/* create a sample file to map onto with mmap */
- fd = syscall(__NR_openat, 0, sample_file, O_RDWR | O_CREAT, FILE_PERM);
- fd = tmpfd();
ASSERT_GE(fd, 0); diff --git a/tools/testing/selftests/arm64/morello/read_write.c b/tools/testing/selftests/arm64/morello/read_write.c index a68b5efeb5ab..6425c8449baa 100644 --- a/tools/testing/selftests/arm64/morello/read_write.c +++ b/tools/testing/selftests/arm64/morello/read_write.c @@ -7,7 +7,6 @@ #include <linux/fs.h> #include <linux/stat.h> #include <linux/uio.h> -#include <asm/fcntl.h> #include <asm/unistd.h> #include "freestanding.h" @@ -26,8 +25,6 @@ static const char w_vec_msg2[VEC_MSG_LEN] = " is vector\n"; static char r_vec_msg1[VEC_MSG_LEN]; static char r_vec_msg2[VEC_MSG_LEN]; -static const char file[] = "/check_cap.txt";
- static const struct iovec w_iovec[VEC_MSG_NUM] = { {.iov_base = (char *) w_vec_msg1, .iov_len = VEC_MSG_LEN}, {.iov_base = (char *) w_vec_msg2, .iov_len = VEC_MSG_LEN},
@@ -58,8 +55,8 @@ TEST(test_writev) TEST(test_open) {
- fd = syscall(__NR_openat, 0, file, O_RDWR | O_CREAT, 0666);
- ASSERT_LE(1, fd) TH_LOG("open failed");
- /* tmpfd() asserts that the openat syscall succeeds */
- fd = tmpfd(); }
TEST(test_read) diff --git a/tools/testing/selftests/arm64/morello/uaccess.c b/tools/testing/selftests/arm64/morello/uaccess.c index a1667d9b6bfc..7a48a35c67c5 100644 --- a/tools/testing/selftests/arm64/morello/uaccess.c +++ b/tools/testing/selftests/arm64/morello/uaccess.c @@ -5,11 +5,8 @@ #include <stddef.h> #include <stdbool.h> -#include <asm/fcntl.h> -#include <asm-generic/errno-base.h> #include <cheriintrin.h> #include <linux/errno.h> -#include <linux/fs.h> #include <linux/futex.h> #include <linux/mman.h> #include <linux/signal.h> @@ -22,14 +19,11 @@ #define MSG_LEN 16 -static const char file[] = "/my_file.txt";
- static inline int fsopen(const char *string) { return syscall(__NR_fsopen, string); }
- static inline int futex_wake_op(uint32_t *uaddr, uint32_t *uaddr2, uint32_t val3) { return syscall(__NR_futex, uaddr, FUTEX_WAKE_OP, 1, 1,
@@ -48,11 +42,6 @@ static int getsockname(int socket, struct sockaddr *sockaddr, int *socklen) return syscall(__NR_getsockname, socket, sockaddr, socklen); } -static int open_file(void) -{
- return syscall(__NR_openat, 0, file, O_RDWR | O_CREAT, 0666);
-}
- static inline int preadv(int fd, struct iovec *iov, int iovcnt) { return syscall(__NR_preadv, fd, iov, iovcnt, 0);
@@ -181,7 +170,7 @@ TEST(test_explicit_iov_iter) int iovcnt = sizeof(iov) / sizeof(struct iovec); int fd;
- fd = open_file();
- fd = tmpfd(); ASSERT_NE(fd, -1);
iov[0].iov_base = (void *)write_buf0;
I've built this locally and it all works as expected on my board when running the tests (all tests passing). Even though I've defined most of the syscall helpers in the kselftest file itself, I think it's fine to keep tmpfd in freestanding.h
Akram
On 22/03/2024 13:29, Akram Ahmad wrote:
I've built this locally and it all works as expected on my board when running the tests (all tests passing). Even though I've defined most of the syscall helpers in the kselftest file itself, I think it's fine to keep tmpfd in freestanding.h
Thank you for the review and testing! Now applied on next.
Note that tmpfd() is used by many tests so it has to be declared in a common header (freestanding.h).
Kevin
linux-morello@op-lists.linaro.org