Accessing elements in an empty va_list is undefined behaviour. Therefore, remove the variadicness from safe_open and safe_openat as they always call open/openat with the mode argument included.
Adapt the SAFE_OPEN and SAFE_OPENAT macros to handle the change by passing a default argument of 0 to mode if it's omitted.
Signed-off-by: Tudor Cretu tudor.cretu@arm.com --- A review branch is created at: https://git.morello-project.org/tudcre01/morello-linux-ltp/-/tree/review/saf...
--- include/old/safe_macros.h | 13 +++++++++++-- include/safe_macros_fn.h | 3 ++- include/tst_safe_file_at.h | 18 ++++++++++++++---- include/tst_safe_macros.h | 12 ++++++++++-- lib/safe_macros.c | 13 +------------ lib/tst_safe_file_at.c | 10 +++------- 6 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/include/old/safe_macros.h b/include/old/safe_macros.h index fb1d7a110..854c087bc 100644 --- a/include/old/safe_macros.h +++ b/include/old/safe_macros.h @@ -59,9 +59,18 @@ #define SAFE_MUNMAP(cleanup_fn, addr, length) \ safe_munmap(__FILE__, __LINE__, (cleanup_fn), (addr), (length))
-#define SAFE_OPEN(cleanup_fn, pathname, oflags, ...) \ +#define _SAFE_OPEN_3(cleanup_fn, pathname, oflags) \ + safe_open(__FILE__, __LINE__, (cleanup_fn), (pathname), (oflags), 0) + +#define _SAFE_OPEN_4(cleanup_fn, pathname, oflags, mode) \ safe_open(__FILE__, __LINE__, (cleanup_fn), (pathname), (oflags), \ - ##__VA_ARGS__) + (mode)) + +#define _SAFE_OPEN_HELPER(_0, _1, _SAFE_OPEN_X, ...) _SAFE_OPEN_X + +#define SAFE_OPEN(cleanup_fn, pathname, oflags, ...) \ + _SAFE_OPEN_HELPER(, ##__VA_ARGS__, _SAFE_OPEN_4, _SAFE_OPEN_3) \ + (cleanup_fn, pathname, oflags, ##__VA_ARGS__)
#define SAFE_PIPE(cleanup_fn, fildes) \ safe_pipe(__FILE__, __LINE__, cleanup_fn, (fildes)) diff --git a/include/safe_macros_fn.h b/include/safe_macros_fn.h index 3df952811..e84759839 100644 --- a/include/safe_macros_fn.h +++ b/include/safe_macros_fn.h @@ -62,7 +62,8 @@ int safe_munmap(const char *file, const int lineno, void (*cleanup_fn)(void), void *addr, size_t length);
int safe_open(const char *file, const int lineno, - void (*cleanup_fn)(void), const char *pathname, int oflags, ...); + void (*cleanup_fn)(void), const char *pathname, int oflags, + mode_t mode);
int safe_pipe(const char *file, const int lineno, void (*cleanup_fn)(void), int fildes[2]); diff --git a/include/tst_safe_file_at.h b/include/tst_safe_file_at.h index 8df34227f..ca51755a6 100644 --- a/include/tst_safe_file_at.h +++ b/include/tst_safe_file_at.h @@ -9,9 +9,19 @@ #include <sys/types.h> #include <stdarg.h>
-#define SAFE_OPENAT(dirfd, path, oflags, ...) \ - safe_openat(__FILE__, __LINE__, \ - (dirfd), (path), (oflags), ## __VA_ARGS__) +#define _SAFE_OPENAT_3(dirfd, path, oflags) \ + safe_openat(__FILE__, __LINE__, \ + (dirfd), (path), (oflags), 0) + +#define _SAFE_OPENAT_4(dirfd, path, oflags, mode) \ + safe_openat(__FILE__, __LINE__, \ + (dirfd), (path), (oflags), (mode)) + +#define _SAFE_OPENAT_HELPER(_0, _1, _SAFE_OPENAT_X, ...) _SAFE_OPENAT_X + +#define SAFE_OPENAT(dirfd, path, oflags, ...) \ + _SAFE_OPENAT_HELPER(, ##__VA_ARGS__, _SAFE_OPENAT_4, _SAFE_OPENAT_3) \ + (dirfd, path, oflags, ##__VA_ARGS__)
#define SAFE_FILE_READAT(dirfd, path, buf, nbyte) \ safe_file_readat(__FILE__, __LINE__, \ @@ -29,7 +39,7 @@ const char *tst_decode_fd(const int fd) __attribute__((warn_unused_result));
int safe_openat(const char *const file, const int lineno, const int dirfd, - const char *const path, const int oflags, ...) + const char *const path, const int oflags, const mode_t mode) __attribute__((nonnull, warn_unused_result));
ssize_t safe_file_readat(const char *const file, const int lineno, diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h index 81c4b0844..9fd51c3ea 100644 --- a/include/tst_safe_macros.h +++ b/include/tst_safe_macros.h @@ -86,9 +86,17 @@ void *safe_realloc(const char *file, const int lineno, void *ptr, size_t size); #define SAFE_MUNMAP(addr, length) \ safe_munmap(__FILE__, __LINE__, NULL, (addr), (length))
+#define _SAFE_OPEN_2(pathname, oflags) \ + safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), 0) + +#define _SAFE_OPEN_3(pathname, oflags, mode) \ + safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), (mode)) + +#define _SAFE_OPEN_HELPER(_0, _1, _SAFE_OPEN_X, ...) _SAFE_OPEN_X + #define SAFE_OPEN(pathname, oflags, ...) \ - safe_open(__FILE__, __LINE__, NULL, (pathname), (oflags), \ - ##__VA_ARGS__) + _SAFE_OPEN_HELPER(, ##__VA_ARGS__, _SAFE_OPEN_3, _SAFE_OPEN_2) \ + (pathname, oflags, ##__VA_ARGS__)
#define SAFE_PIPE(fildes) \ safe_pipe(__FILE__, __LINE__, NULL, (fildes)) diff --git a/lib/safe_macros.c b/lib/safe_macros.c index a5b6bc504..40891e841 100644 --- a/lib/safe_macros.c +++ b/lib/safe_macros.c @@ -234,20 +234,9 @@ 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, ...) + const char *pathname, int oflags, mode_t mode) { - va_list ap; int rval; - mode_t mode; - - 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);
rval = open(pathname, oflags, mode);
diff --git a/lib/tst_safe_file_at.c b/lib/tst_safe_file_at.c index ca8ef2f68..1c1f646bc 100644 --- a/lib/tst_safe_file_at.c +++ b/lib/tst_safe_file_at.c @@ -33,15 +33,11 @@ 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, ...) + const int dirfd, const char *const path, const int oflags, + const mode_t mode) { va_list ap; int fd; - mode_t mode; - - va_start(ap, oflags); - mode = va_arg(ap, int); - va_end(ap);
fd = openat(dirfd, path, oflags, mode); if (fd > -1) @@ -58,7 +54,7 @@ ssize_t safe_file_readat(const char *const file, const int lineno, const int dirfd, const char *const path, char *const buf, const size_t nbyte) { - int fd = safe_openat(file, lineno, dirfd, path, O_RDONLY); + int fd = safe_openat(file, lineno, dirfd, path, O_RDONLY, 0); ssize_t rval;
if (fd < 0)