* [LTP] [PATCH v4 1/3] syscalls/f{get, set}xattr: Don't pass a pointer to mode argument in open
2022-11-30 15:07 [LTP] [PATCH v4 0/3] safe_macros: Fix undefined behaviour in vararg handling Tudor Cretu
@ 2022-11-30 15:07 ` Tudor Cretu
2022-11-30 15:07 ` [LTP] [PATCH v4 2/3] safe_open, safe_openat: Fix undefined behaviour in vararg handling Tudor Cretu
2022-11-30 15:07 ` [LTP] [PATCH v4 3/3] safe_semctl: " Tudor Cretu
2 siblings, 0 replies; 6+ messages in thread
From: Tudor Cretu @ 2022-11-30 15:07 UTC (permalink / raw)
To: ltp
Passing NULL to open as the mode parameter is undefined behaviour
as a mode_t is a numerical type, not compatible with a pointer type.
If neither O_CREAT nor O_TMPFILE is specified in flags, then the mode
argument is ignored, so just remove it in these cases.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
---
testcases/kernel/syscalls/fgetxattr/fgetxattr01.c | 2 +-
testcases/kernel/syscalls/fgetxattr/fgetxattr02.c | 2 +-
testcases/kernel/syscalls/fgetxattr/fgetxattr03.c | 2 +-
testcases/kernel/syscalls/fsetxattr/fsetxattr01.c | 2 +-
testcases/kernel/syscalls/fsetxattr/fsetxattr02.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/testcases/kernel/syscalls/fgetxattr/fgetxattr01.c b/testcases/kernel/syscalls/fgetxattr/fgetxattr01.c
index 35c46a1c3..52e6e44ab 100644
--- a/testcases/kernel/syscalls/fgetxattr/fgetxattr01.c
+++ b/testcases/kernel/syscalls/fgetxattr/fgetxattr01.c
@@ -111,7 +111,7 @@ static void setup(void)
size_t i = 0;
SAFE_TOUCH(FNAME, 0644, NULL);
- fd = SAFE_OPEN(FNAME, O_RDONLY, NULL);
+ fd = SAFE_OPEN(FNAME, O_RDONLY);
for (i = 0; i < ARRAY_SIZE(tc); i++) {
tc[i].value = SAFE_MALLOC(tc[i].size);
diff --git a/testcases/kernel/syscalls/fgetxattr/fgetxattr02.c b/testcases/kernel/syscalls/fgetxattr/fgetxattr02.c
index 82fb676be..c3cff0aab 100644
--- a/testcases/kernel/syscalls/fgetxattr/fgetxattr02.c
+++ b/testcases/kernel/syscalls/fgetxattr/fgetxattr02.c
@@ -244,7 +244,7 @@ static void setup(void)
SAFE_BIND(tc[i].fd, (const struct sockaddr *) &sun,
sizeof(struct sockaddr_un));
} else {
- tc[i].fd = SAFE_OPEN(tc[i].fname, tc[i].fflags, NULL);
+ tc[i].fd = SAFE_OPEN(tc[i].fname, tc[i].fflags);
}
if (tc[i].exp_ret >= 0) {
diff --git a/testcases/kernel/syscalls/fgetxattr/fgetxattr03.c b/testcases/kernel/syscalls/fgetxattr/fgetxattr03.c
index d293ffca9..0581b9670 100644
--- a/testcases/kernel/syscalls/fgetxattr/fgetxattr03.c
+++ b/testcases/kernel/syscalls/fgetxattr/fgetxattr03.c
@@ -48,7 +48,7 @@ static void verify_fgetxattr(void)
static void setup(void)
{
SAFE_TOUCH(FILENAME, 0644, NULL);
- fd = SAFE_OPEN(FILENAME, O_RDONLY, NULL);
+ fd = SAFE_OPEN(FILENAME, O_RDONLY);
SAFE_FSETXATTR(fd, XATTR_TEST_KEY, XATTR_TEST_VALUE,
XATTR_TEST_VALUE_SIZE, XATTR_CREATE);
diff --git a/testcases/kernel/syscalls/fsetxattr/fsetxattr01.c b/testcases/kernel/syscalls/fsetxattr/fsetxattr01.c
index ffec8104f..d799e477f 100644
--- a/testcases/kernel/syscalls/fsetxattr/fsetxattr01.c
+++ b/testcases/kernel/syscalls/fsetxattr/fsetxattr01.c
@@ -205,7 +205,7 @@ static void setup(void)
long_value[XATTR_SIZE_MAX + 1] = '\0';
SAFE_TOUCH(FNAME, 0644, NULL);
- fd = SAFE_OPEN(FNAME, O_RDONLY, NULL);
+ fd = SAFE_OPEN(FNAME, O_RDONLY);
for (i = 0; i < ARRAY_SIZE(tc); i++) {
if (!tc[i].key)
diff --git a/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c b/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c
index 3aea4b59e..0336c964a 100644
--- a/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c
+++ b/testcases/kernel/syscalls/fsetxattr/fsetxattr02.c
@@ -211,7 +211,7 @@ static void setup(void)
for (i = 0; i < ARRAY_SIZE(tc); i++) {
if (!tc[i].issocket) {
- tc[i].fd = SAFE_OPEN(tc[i].fname, tc[i].fflags, NULL);
+ tc[i].fd = SAFE_OPEN(tc[i].fname, tc[i].fflags);
continue;
}
--
2.25.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread* [LTP] [PATCH v4 2/3] safe_open, safe_openat: Fix undefined behaviour in vararg handling
2022-11-30 15:07 [LTP] [PATCH v4 0/3] safe_macros: Fix undefined behaviour in vararg handling Tudor Cretu
2022-11-30 15:07 ` [LTP] [PATCH v4 1/3] syscalls/f{get, set}xattr: Don't pass a pointer to mode argument in open Tudor Cretu
@ 2022-11-30 15:07 ` Tudor Cretu
2022-11-30 15:25 ` Cyril Hrubis
2022-11-30 15:07 ` [LTP] [PATCH v4 3/3] safe_semctl: " Tudor Cretu
2 siblings, 1 reply; 6+ messages in thread
From: Tudor Cretu @ 2022-11-30 15:07 UTC (permalink / raw)
To: ltp
Accessing elements in an empty va_list is undefined behaviour.
The open system call expects the mode argument only when O_CREAT or
O_TMPFILE is specified in flags, otherwise the mode argument is ignored.
Modify the safe_open and safe_openat wrappers to read the variadic
argument only when it's expected to be provided.
Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
---
include/lapi/fcntl.h | 3 +++
lib/safe_macros.c | 21 ++++++++++++---------
lib/tst_safe_file_at.c | 13 ++++++++-----
3 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
index 8a0a893f9..f17220737 100644
--- a/include/lapi/fcntl.h
+++ b/include/lapi/fcntl.h
@@ -141,6 +141,9 @@
# define MAX_HANDLE_SZ 128
#endif
+#define TST_OPEN_NEEDS_MODE(oflag) \
+ (((oflag) & O_CREAT) != 0 || ((oflag) & O_TMPFILE) == O_TMPFILE)
+
#ifndef HAVE_STRUCT_FILE_HANDLE
struct file_handle {
unsigned int handle_bytes;
diff --git a/lib/safe_macros.c b/lib/safe_macros.c
index d8816631f..d34b71fda 100644
--- a/lib/safe_macros.c
+++ b/lib/safe_macros.c
@@ -13,7 +13,6 @@
#include <sys/xattr.h>
#include <sys/sysinfo.h>
#include <errno.h>
-#include <fcntl.h>
#include <libgen.h>
#include <limits.h>
#include <pwd.h>
@@ -21,6 +20,7 @@
#include <stdlib.h>
#include <unistd.h>
#include <malloc.h>
+#include "lapi/fcntl.h"
#include "test.h"
#include "safe_macros.h"
@@ -236,18 +236,21 @@ int safe_munmap(const char *file, const int lineno, void (*cleanup_fn) (void),
int safe_open(const char *file, const int lineno, void (*cleanup_fn) (void),
const char *pathname, int oflags, ...)
{
- va_list ap;
int rval;
- mode_t mode;
+ mode_t mode = 0;
- va_start(ap, oflags);
+ if (TST_OPEN_NEEDS_MODE(oflags)) {
+ va_list ap;
- /* Android's NDK's mode_t is smaller than an int, which results in
- * SIGILL here when passing the mode_t type.
- */
- mode = va_arg(ap, int);
+ va_start(ap, oflags);
+
+ /* Android's NDK's mode_t is smaller than an int, which results in
+ * SIGILL here when passing the mode_t type.
+ */
+ mode = va_arg(ap, int);
- va_end(ap);
+ va_end(ap);
+ }
rval = open(pathname, oflags, mode);
diff --git a/lib/tst_safe_file_at.c b/lib/tst_safe_file_at.c
index f530dc349..257f70291 100644
--- a/lib/tst_safe_file_at.c
+++ b/lib/tst_safe_file_at.c
@@ -35,13 +35,16 @@ const char *tst_decode_fd(const int fd)
int safe_openat(const char *const file, const int lineno,
const int dirfd, const char *const path, const int oflags, ...)
{
- va_list ap;
int fd;
- mode_t mode;
+ mode_t mode = 0;
- va_start(ap, oflags);
- mode = va_arg(ap, int);
- va_end(ap);
+ if (TST_OPEN_NEEDS_MODE(oflags)) {
+ va_list ap;
+
+ va_start(ap, oflags);
+ mode = va_arg(ap, int);
+ va_end(ap);
+ }
fd = openat(dirfd, path, oflags, mode);
if (fd > -1)
--
2.25.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread* [LTP] [PATCH v4 3/3] safe_semctl: Fix undefined behaviour in vararg handling
2022-11-30 15:07 [LTP] [PATCH v4 0/3] safe_macros: Fix undefined behaviour in vararg handling Tudor Cretu
2022-11-30 15:07 ` [LTP] [PATCH v4 1/3] syscalls/f{get, set}xattr: Don't pass a pointer to mode argument in open Tudor Cretu
2022-11-30 15:07 ` [LTP] [PATCH v4 2/3] safe_open, safe_openat: Fix undefined behaviour in vararg handling Tudor Cretu
@ 2022-11-30 15:07 ` Tudor Cretu
2 siblings, 0 replies; 6+ messages in thread
From: Tudor Cretu @ 2022-11-30 15:07 UTC (permalink / raw)
To: ltp
Accessing elements in an empty va_list is undefined behaviour.
The semctl system call expects the union semun argument only for a
set of cmd values, otherwise the argument is ignored.
Modify the safe_semctl wrapper to read the variadic argument only when
it's expected to be provided.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
---
include/lapi/ipc.h | 14 ++++++++++++++
lib/tst_safe_sysv_ipc.c | 23 ++++++++++++++++-------
2 files changed, 30 insertions(+), 7 deletions(-)
create mode 100644 include/lapi/ipc.h
diff --git a/include/lapi/ipc.h b/include/lapi/ipc.h
new file mode 100644
index 000000000..5645c8817
--- /dev/null
+++ b/include/lapi/ipc.h
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2022 Arm Ltd.
+ */
+#ifndef LAPI_IPC_H__
+#define LAPI_IPC_H__
+
+#include <sys/ipc.h>
+
+#ifndef IPC_INFO
+# define IPC_INFO 3
+#endif
+
+#endif /* LAPI_IPC_H__ */
diff --git a/lib/tst_safe_sysv_ipc.c b/lib/tst_safe_sysv_ipc.c
index 5eaa82539..a196fc9ce 100644
--- a/lib/tst_safe_sysv_ipc.c
+++ b/lib/tst_safe_sysv_ipc.c
@@ -4,12 +4,12 @@
*/
#include <sys/types.h>
-#include <sys/ipc.h>
#include <sys/msg.h>
#include <sys/shm.h>
#define TST_NO_DEFAULT_MAIN
#include "tst_test.h"
#include "tst_safe_sysv_ipc.h"
+#include "lapi/ipc.h"
#include "lapi/sem.h"
/*
@@ -232,13 +232,22 @@ int safe_semctl(const char *file, const int lineno, int semid, int semnum,
{
int rval;
va_list va;
- union semun un;
+ union semun un = {0};
- va_start(va, cmd);
-
- un = va_arg(va, union semun);
-
- va_end(va);
+ switch (cmd) {
+ case SETVAL:
+ case GETALL:
+ case SETALL:
+ case IPC_STAT:
+ case IPC_SET:
+ case SEM_STAT:
+ case SEM_STAT_ANY:
+ case IPC_INFO:
+ case SEM_INFO:
+ va_start(va, cmd);
+ un = va_arg(va, union semun);
+ va_end(va);
+ }
rval = semctl(semid, semnum, cmd, un);
--
2.25.1
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 6+ messages in thread