Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Mon, 19 Jun 2017 21:10:43 +0300
From: Omer Anson <oaanson@...il.com>
To: musl@...ts.openwall.com
Cc: Omer Anson <oaanson@...il.com>
Subject: [PATCH] Be more precise using va_arg in fcntl

Currently, fcntl automatically calls va_arg with type unsigned long,
regardless of the actual invoked command, and therefore ignoring whether
such an argument exists, or its type.

According to the C standard[1][2], calling va_arg if the types do not match,
or beyond the end of the parameter list, causes an undefined behaviour.

This change modifies fcntl to try and call va_arg only when necessary,
and with the correct parameter type. It relies on the cmd argument to
know what's the expected command. In case the cmd is unknown, it falls
back to the previous behaviour.

[1] http://www.open-std.org/jtc1/sc22/wg14/www/standards
[2] http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/stdarg.h.html
---
 src/fcntl/fcntl.c | 72 ++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 16 deletions(-)

diff --git a/src/fcntl/fcntl.c b/src/fcntl/fcntl.c
index ce615d0..9d6b2cc 100644
--- a/src/fcntl/fcntl.c
+++ b/src/fcntl/fcntl.c
@@ -5,24 +5,40 @@
 #include "syscall.h"
 #include "libc.h"
 
+#define GET_ARG_MACRO(type) ({ \
+		va_list ap; \
+		va_start(ap, cmd); \
+		type __arg = va_arg(ap, type); \
+		va_end(ap); \
+		__arg; \
+		})
+
 int fcntl(int fd, int cmd, ...)
 {
-	unsigned long arg;
-	va_list ap;
-	va_start(ap, cmd);
-	arg = va_arg(ap, unsigned long);
-	va_end(ap);
-	if (cmd == F_SETFL) arg |= O_LARGEFILE;
-	if (cmd == F_SETLKW) return syscall_cp(SYS_fcntl, fd, cmd, (void *)arg);
-	if (cmd == F_GETOWN) {
+	switch (cmd) {
+	case F_GETFD:
+	case F_GETFL:
+	case F_GETSIG:
+	case F_GETLEASE:
+	case F_GET_SEALS:
+	case F_GETPIPE_SZ:
+		return syscall(SYS_fcntl, fd, cmd);
+	case F_GETOWN: {
 		struct f_owner_ex ex;
 		int ret = __syscall(SYS_fcntl, fd, F_GETOWN_EX, &ex);
-		if (ret == -EINVAL) return __syscall(SYS_fcntl, fd, cmd, (void *)arg);
+		if (ret == -EINVAL) return __syscall(SYS_fcntl, fd, cmd);
 		if (ret) return __syscall_ret(ret);
 		return ex.type == F_OWNER_PGRP ? -ex.pid : ex.pid;
 	}
-	if (cmd == F_DUPFD_CLOEXEC) {
-		int ret = __syscall(SYS_fcntl, fd, F_DUPFD_CLOEXEC, arg);
+	case F_SETFL: {
+		int arg = GET_ARG_MACRO(int);
+		arg |= O_LARGEFILE;
+		return syscall(SYS_fcntl, fd, cmd, arg);
+	}
+	case F_DUPFD_CLOEXEC: {
+		int ret;
+		int arg = GET_ARG_MACRO(int);
+		ret = __syscall(SYS_fcntl, fd, F_DUPFD_CLOEXEC, arg);
 		if (ret != -EINVAL) {
 			if (ret >= 0)
 				__syscall(SYS_fcntl, ret, F_SETFD, FD_CLOEXEC);
@@ -37,13 +53,37 @@ int fcntl(int fd, int cmd, ...)
 		if (ret >= 0) __syscall(SYS_fcntl, ret, F_SETFD, FD_CLOEXEC);
 		return __syscall_ret(ret);
 	}
-	switch (cmd) {
-	case F_SETLK:
+	case F_DUPFD:
+	case F_SETFD:
+	case F_SETSIG:
+	case F_SETLEASE:
+	case F_ADD_SEALS:
+	case F_SETOWN:
+	case F_SETPIPE_SZ:
+	case F_NOTIFY: {
+		int arg = GET_ARG_MACRO(int);
+		return syscall(SYS_fcntl, fd, cmd, arg);
+	}
 	case F_GETLK:
+	case F_SETLK:
+	case F_OFD_SETLK:
+	case F_OFD_GETLK: {
+		struct flock * arg = GET_ARG_MACRO(struct flock *);
+		return syscall(SYS_fcntl, fd, cmd, arg);
+	}
+	case F_OFD_SETLKW:
+	case F_SETLKW: {
+		struct flock * arg = GET_ARG_MACRO(struct flock *);
+		return syscall_cp(SYS_fcntl, fd, cmd, arg);
+	}
 	case F_GETOWN_EX:
-	case F_SETOWN_EX:
-		return syscall(SYS_fcntl, fd, cmd, (void *)arg);
-	default:
+	case F_SETOWN_EX: {
+		struct f_owner_ex * arg = GET_ARG_MACRO(struct f_owner_ex *);
 		return syscall(SYS_fcntl, fd, cmd, arg);
 	}
+	default: {
+		unsigned long arg = GET_ARG_MACRO(unsigned long);
+		return syscall(SYS_fcntl, fd, cmd, arg);
+	}
+	}
 }
-- 
2.4.11

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.