[LTP] [PATCH v3 2/3] safe_open, safe_openat: Fix undefined behaviour in vararg handling

[Tudor Cretu]

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   |  5 +++++
 lib/safe_macros.c      | 21 ++++++++++++---------
 lib/tst_safe_file_at.c | 13 ++++++++-----
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/include/lapi/fcntl.h b/include/lapi/fcntl.h
index 8a0a893f9..c99ceb08d 100644
--- a/include/lapi/fcntl.h
+++ b/include/lapi/fcntl.h
@@ -141,6 +141,11 @@
 # define MAX_HANDLE_SZ	128
 #endif
 
+#ifndef __OPEN_NEEDS_MODE
+ #define __OPEN_NEEDS_MODE(oflag) \
+	(((oflag) & O_CREAT) != 0 || ((oflag) & O_TMPFILE) == O_TMPFILE)
+#endif
+
 #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..af00db960 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 (__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..6d0925b74 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 (__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