Safely interpose fcntl() on architectures with bounded variadic argum…

…ents

`fcntl()` abuses C variadic arguments to implement optional parameters.
We want to forward this argument to the real `fcntl()` call, but doing
so is non-portable. On most architectures, we can just read an
`intptr_t`/`void*` and this will give us a (semi-)valid result even if
no argument/an int was passed instead since `va_arg()` will generally
read the next argument register or arbitrary data from the stack.

On CHERI-enabled architectures, variadic arguments are tightly
bounded, which means that reading a `void*` if an `int` was passed will
result in a runtime trap. To avoid this problem this commit uses a new
`vfcntl` libc function (CTSRD-CHERI/cheribsd#2194),
that takes a va_list instead of variadic arguments.

Since this needs a new HAVE_VCNTL compile-time check, this adds a new
config.h header that also includes the HAVE_TIMERFD value. This removes
the need to pass -D flags on the command line.

This reduces the number of test failures on CheriBSD Morello from 210
to 8.

src/CMakeLists.txt

@@ -25,6 +25,7 @@ add_library(wrap OBJECT wrap.c)
 set_property(TARGET wrap PROPERTY POSITION_INDEPENDENT_CODE ON)
 target_link_libraries(wrap PUBLIC Threads::Threads)
 target_include_directories(wrap
+                           PRIVATE $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
                            PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}>)
 
 macro(add_compat_target _name _condition)
@@ -51,6 +52,8 @@ include(CheckSymbolExists)
 # supports native timerfd descriptors. Prefer them if available.
 check_symbol_exists(eventfd "sys/eventfd.h" HAVE_EVENTFD)
 check_symbol_exists(timerfd_create "sys/timerfd.h" HAVE_TIMERFD)
+# Presence of a variadic version of fcntl allows for safer interposing.
+check_symbol_exists(vfcntl "fcntl.h;stdarg.h" HAVE_VFCNTL)
 
 check_symbol_exists(kqueue1 "sys/types.h;sys/event.h;sys/time.h" HAVE_KQUEUE1)
 add_compat_target(kqueue1 "NOT;HAVE_KQUEUE1")
@@ -117,9 +120,9 @@ target_link_libraries(
           $<BUILD_INTERFACE:compat_enable_sigops>
           $<BUILD_INTERFACE:rwlock>
           $<BUILD_INTERFACE:wrap>)
-if(HAVE_TIMERFD)
-  target_compile_definitions(epoll-shim PRIVATE HAVE_TIMERFD)
-endif()
+configure_file(
+  epoll_shim_config.h.cmake
+  ${CMAKE_CURRENT_BINARY_DIR}/epoll_shim_config.h)
 target_compile_definitions(epoll-shim PRIVATE EPOLL_SHIM_DISABLE_WRAPPER_MACROS)
 target_include_directories(
   epoll-shim

src/epoll_shim_config.h.cmake

@@ -0,0 +1,3 @@
+#cmakedefine01 HAVE_TIMERFD
+#cmakedefine01 HAVE_VFCNTL
+

src/epoll_shim_ctx.c

@@ -20,6 +20,7 @@
 #include <time.h>
 #include <unistd.h>
 
+#include "epoll_shim_config.h"
 #include "epoll_shim_export.h"
 #include "errno_return.h"
 #include "timespec_util.h"
@@ -427,7 +428,7 @@ epoll_shim_ctx_drop_desc(EpollShimCtx *epoll_shim_ctx, /**/
 	rwlock_unlock_write(&epoll_shim_ctx->rwlock);
 }
 
-#ifndef HAVE_TIMERFD
+#if !HAVE_TIMERFD
 static void
 trigger_realtime_change_notification(FileDescription *desc, int kq, void *arg)
 {
@@ -792,30 +793,34 @@ epoll_shim_ppoll(struct pollfd *fds, nfds_t nfds, struct timespec const *tmo_p,
 EPOLL_SHIM_EXPORT
 int
 epoll_shim_fcntl(int fd, int cmd, ...)
+{
+	va_list ap;
+	va_start(ap, cmd);
+	int result = epoll_shim_vfcntl(fd, cmd, ap);
+	va_end(ap);
+	return result;
+}
+
+EPOLL_SHIM_EXPORT
+int
+epoll_shim_vfcntl(int fd, int cmd, va_list ap)
 {
 	ERRNO_SAVE;
 
 	EpollShimCtx *epoll_shim_ctx;
 	FileDescription *desc;
-	va_list ap;
 
 	if (fd < 0 || (cmd != F_SETFL && cmd != F_GETFL) ||
 	    epoll_shim_ctx_global(&epoll_shim_ctx) != 0 ||
 	    (desc = epoll_shim_ctx_find_desc(epoll_shim_ctx, fd)) == NULL) {
-		va_start(ap, cmd);
-		void *arg = va_arg(ap, void *);
-		va_end(ap);
-
-		ERRNO_RETURN(0, -1, real_fcntl(fd, cmd, arg));
+		ERRNO_RETURN(0, -1, real_vfcntl(fd, cmd, ap));
 	}
 
 	errno_t ec;
 	int result = 0;
 
 	if (cmd == F_SETFL) {
-		va_start(ap, cmd);
 		int arg = va_arg(ap, int);
-		va_end(ap);
 
 		(void)pthread_mutex_lock(&desc->mutex);
 		{

src/epoll_shim_ctx.h

@@ -4,6 +4,7 @@
 #include <sys/tree.h>
 
 #include <stdatomic.h>
+#include <stdarg.h>
 
 #include <signal.h>
 #include <unistd.h>
@@ -83,5 +84,6 @@ int epoll_shim_ppoll(struct pollfd *, nfds_t, struct timespec const *,
     sigset_t const *);
 
 int epoll_shim_fcntl(int fd, int cmd, ...);
+int epoll_shim_vfcntl(int fd, int cmd, va_list);
 
 #endif

src/epoll_shim_interpose.c

@@ -6,14 +6,7 @@
 #include <unistd.h>
 
 #include "epoll_shim_interpose_export.h"
-
-int epoll_shim_close(int fd);
-ssize_t epoll_shim_read(int fd, void *buf, size_t nbytes);
-ssize_t epoll_shim_write(int fd, void const *buf, size_t nbytes);
-int epoll_shim_poll(struct pollfd *, nfds_t, int);
-int epoll_shim_ppoll(struct pollfd *, nfds_t, struct timespec const *,
-    sigset_t const *);
-int epoll_shim_fcntl(int fd, int cmd, ...);
+#include "epoll_shim_ctx.h"
 
 EPOLL_SHIM_INTERPOSE_EXPORT
 ssize_t
@@ -65,8 +58,7 @@ fcntl(int fd, int cmd, ...)
 	va_list ap;
 
 	va_start(ap, cmd);
-	void *arg = va_arg(ap, void *);
-	int rv = epoll_shim_fcntl(fd, cmd, arg);
+	int rv = epoll_shim_vfcntl(fd, cmd, ap);
 	va_end(ap);
 
 	return rv;

src/wrap.c

@@ -12,6 +12,7 @@
 #include <unistd.h>
 
 #include "compat_ppoll.h"
+#include "epoll_shim_config.h"
 
 static struct {
 	pthread_once_t wrap_init;
@@ -27,7 +28,11 @@ static struct {
 	typeof(ppoll) *real_ppoll;
 #endif
 #endif
+#if !HAVE_VFCNTL
 	typeof(fcntl) *real_fcntl;
+#else
+	typeof(vfcntl) *real_vfcntl;
+#endif
 } wrap = { .wrap_init = PTHREAD_ONCE_INIT };
 
 static void
@@ -65,7 +70,11 @@ wrap_initialize_impl(void)
 	WRAP(ppoll);
 #endif
 #endif
+#if !HAVE_VFCNTL
 	WRAP(fcntl);
+#else
+	WRAP(vfcntl);
+#endif
 
 #undef WRAP
 }
@@ -124,14 +133,42 @@ real_ppoll(struct pollfd fds[], nfds_t nfds,
 }
 
 int
-real_fcntl(int fd, int cmd, ...)
+real_vfcntl(int fd, int cmd, va_list ap)
 {
 	wrap_initialize();
+
+	// fcntl() abuses C variadic arguments to implement optional parameters.
+	// We want to forward this argument to the real fcntl() call, but doing
+	// so is non-portable. On most architectures, we can just read an
+	// intptr_t/void* and this will give us a (semi-)valid result even if
+	// no argument/an int was passed instead since va_arg() will generally
+	// read the next argument register or arbitrary data from the stack.
+	//
+	// On CHERI-enabled architectures, variadic arguments are tightly
+	// bounded, which means that reading an 8-byte void* if a 4-byte int was
+	// passed will result in a runtime trap. Newer CheriBSD systems support
+	// a vfcntl() function that can be used to safely forward arguments to
+	// fcntl() without having to duplicate the logic to determine how many
+	// arguments will be read.
+#if HAVE_VFCNTL
+	// vfcntl() is present, can just foward the va_list.
+	return wrap.real_vfcntl(fd, cmd, ap);
+#else
+	// Otherwise, we assume that we can read a void* from the va_list
+	// even if the actual value passed was smaller or a different type and
+	// hope that this will match the calling convention.
+	void *arg = va_arg(ap, void *);
+	return wrap.real_fcntl(fd, cmd, arg);
+#endif
+}
+
+int
+real_fcntl(int fd, int cmd, ...)
+{
 	va_list ap;
 
 	va_start(ap, cmd);
-	void *arg = va_arg(ap, void *);
-	int rv = wrap.real_fcntl(fd, cmd, arg);
+	int rv = real_vfcntl(fd, cmd, ap);
 	va_end(ap);
 
 	return rv;

src/wrap.h

@@ -3,6 +3,7 @@
 
 #include <poll.h>
 #include <signal.h>
+#include <stdarg.h>
 #include <stddef.h>
 #include <time.h>
 #include <unistd.h>
@@ -15,5 +16,6 @@ int real_ppoll(struct pollfd fds[], nfds_t nfds,
     struct timespec const *restrict timeout,
     sigset_t const *restrict newsigmask);
 int real_fcntl(int fd, int cmd, ...);
+int real_vfcntl(int fd, int cmd, va_list ap);
 
 #endif