Better detection of error codes in syscall return values

It was problematic before, especially e.g. the case in xmmap_anon:
the function checked for MMAP_FAILED, but that is only correct
while using the libc mmap interface. The raw syscall on Linux can
return one of several negative integers to indicate errors.

Author: GBuella

doc/generated/libsyscall_intercept.3

@@ -98,17 +98,32 @@ after a clone syscall creating a thread returns in a child thread:
 .IP
 .nf
 \f[C]
-long\ (*intercept_hook_point_clone_child)(void);
+void\ (*intercept_hook_point_clone_child)(void);
 \f[]
 .fi
 .PP
 Using \f[C]intercept_hook_point_clone_child\f[], one can be notified of
-thread creations easily, and even override the syscall\[aq]s return
-value in the child thread (which is normally zero).
-Note: overriding the zero return value is discouraged \-\- this syscall
-is usually issued by libc, and a non\-zero return value is interpreted
-as the execution remaining in the parent thread, thus using attempting
-to use the same stack space as the actual parent thread.
+thread creations.
+.PP
+To make it easy to detect syscall return values indicating errors, one
+can use the syscall_error_code function:
+.IP
+.nf
+\f[C]
+int\ syscall_error_code(long\ result);
+\f[]
+.fi
+.PP
+When passed a return value from syscall_no_intercept, this function can
+translate it to an error code equivalent to a libc error code:
+.IP
+.nf
+\f[C]
+int\ fd\ =\ (int)syscall_no_intercept(SYS_open,\ "file",\ O_RDWR);
+if\ (syscall_error_code(fd)\ !=\ 0)
+\ \ \ \ fprintf(stderr,\ strerror(syscall_error_code(fd)));
+\f[]
+.fi
 .SH ENVIRONMENT VARIABLES
 .PP
 Three environment variables control the operation of the library:

doc/libsyscall_intercept.3.md

@@ -101,6 +101,19 @@ void (*intercept_hook_point_clone_child)(void);
 Using `intercept_hook_point_clone_child`, one can be notified of thread
 creations.
 
+To make it easy to detect syscall return values indicating errors, one
+can use the syscall_error_code function:
+```c
+int syscall_error_code(long result);
+```
+When passed a return value from syscall_no_intercept, this function
+can translate it to an error code equivalent to a libc error code:
+```c
+int fd = (int)syscall_no_intercept(SYS_open, "file", O_RDWR);
+if (syscall_error_code(fd) != 0)
+	fprintf(stderr, strerror(syscall_error_code(fd)));
+```
+
 # ENVIRONMENT VARIABLES #
 Three environment variables control the operation of the library:
 

include/libsyscall_intercept_hook_point.h

@@ -69,6 +69,20 @@ extern void (*intercept_hook_point_clone_child)(void);
  */
 long syscall_no_intercept(long syscall_number, ...);
 
+/*
+ * syscall_error_code - examines a return value from
+ * syscall_no_intercept, and returns an error code if said
+ * return value indicates an error.
+ */
+static inline int
+syscall_error_code(long result)
+{
+	if (result < 0 && result >= -0x1000)
+		return (int)-result;
+
+	return 0;
+}
+
 /*
  * The syscall intercepting library checks for the
  * INTERCEPT_HOOK_CMDLINE_FILTER environment variable, with which one can

src/intercept.c

@@ -448,21 +448,65 @@ log_header(void)
 }
 
 /*
- * xabort - speaks for itself
+ * xabort_errno - print a message to stderr, and exit the process.
  * Calling abort() in libc might result other syscalls being called
  * by libc.
+ *
+ * If error_code is not zero, it is also printed.
  */
 void
-xabort(const char *msg)
+xabort_errno(int error_code, const char *msg)
 {
 	static const char main_msg[] = " libsyscall_intercept error\n";
 
-	if (msg != NULL)
-		syscall_no_intercept(SYS_write, 2, msg, strlen(msg));
-	syscall_no_intercept(SYS_write, 2, main_msg, sizeof(main_msg));
+	if (msg != NULL) {
+		/* not using libc - inline strlen */
+		size_t len = 0;
+		while (msg[len] != '\0')
+			++len;
+		syscall_no_intercept(SYS_write, 2, msg, len);
+	}
+
+	if (error_code != 0) {
+		char buf[0x10];
+		size_t len = 1;
+		char *c = buf + sizeof(buf) - 1;
+
+		/* not using libc - inline sprintf */
+		do {
+			*c-- = error_code % 10;
+			++len;
+			error_code /= 10;
+		} while (error_code != 0);
+		*c = ' ';
+
+		syscall_no_intercept(SYS_write, 2, c, len);
+	}
+
+	syscall_no_intercept(SYS_write, 2, main_msg, sizeof(main_msg) - 1);
 	syscall_no_intercept(SYS_exit_group, 1);
 
-	__builtin_trap();
+	__builtin_unreachable();
+}
+
+/*
+ * xabort - print a message to stderr, and exit the process.
+ */
+void
+xabort(const char *msg)
+{
+	xabort_errno(0, msg);
+}
+
+/*
+ * xabort_on_syserror -- examines the return value of syscall_no_intercept,
+ * and calls xabort_errno if the said return value indicates an error.
+ */
+void
+xabort_on_syserror(long syscall_result, const char *msg)
+{
+	if (syscall_error_code(syscall_result) != 0)
+		xabort_errno(syscall_error_code(syscall_result), msg);
 }
 
 /*

src/intercept.h

@@ -62,7 +62,11 @@ void intercept_patch_with_postfix(unsigned char *syscall_addr,
 
 #define INTERCEPTOR_EXIT_CODE 111
 
-__attribute__((noreturn)) void xabort(const char *);
+__attribute__((noreturn)) void xabort_errno(int error_code, const char *msg);
+
+__attribute__((noreturn)) void xabort(const char *msg);
+
+void xabort_on_syserror(long syscall_result, const char *msg);
 
 struct range {
 	unsigned char *address;

src/intercept_desc.c

@@ -64,11 +64,7 @@ open_orig_file(const struct intercept_desc *desc)
 
 	fd = syscall_no_intercept(SYS_open, desc->path, O_RDONLY);
 
-	if (fd < 0) {
-		syscall_no_intercept(SYS_write, 2,
-		    desc->path, strlen(desc->path));
-		xabort(" open_orig_file");
-	}
+	xabort_on_syserror(fd, __func__);
 
 	return fd;
 }

src/intercept_util.c

@@ -32,6 +32,7 @@
 
 #include "intercept_util.h"
 #include "intercept.h"
+#include "libsyscall_intercept_hook_point.h"
 
 #include <assert.h>
 #include <inttypes.h>
@@ -53,53 +54,52 @@ static int log_fd = -1;
 void *
 xmmap_anon(size_t size)
 {
-	void *addr = (void *) syscall_no_intercept(SYS_mmap,
+	long addr = syscall_no_intercept(SYS_mmap,
 				NULL, size,
 				PROT_READ | PROT_WRITE,
 				MAP_PRIVATE | MAP_ANON, -1, (off_t)0);
 
-	if (addr == MAP_FAILED)
-		xabort("xmmap_anon");
+	xabort_on_syserror(addr, __func__);
 
-	return addr;
+	return (void *) addr;
 }
 
 void *
 xmremap(void *addr, size_t old, size_t new)
 {
-	addr = (void *) syscall_no_intercept(SYS_mremap, addr,
+	long new_addr = syscall_no_intercept(SYS_mremap, addr,
 				old, new, MREMAP_MAYMOVE);
 
-	if (addr == MAP_FAILED)
-		xabort("xmremap");
+	xabort_on_syserror(new_addr, __func__);
 
-	return addr;
+	return (void *) new_addr;
 }
 
 void
 xmunmap(void *addr, size_t len)
 {
-	if (syscall_no_intercept(SYS_munmap, addr, len) != 0)
-		xabort("xmunmap");
+	long result = syscall_no_intercept(SYS_munmap, addr, len);
+
+	xabort_on_syserror(result, __func__);
 }
 
 long
 xlseek(long fd, unsigned long off, int whence)
 {
 	long result = syscall_no_intercept(SYS_lseek, fd, off, whence);
 
-	if (result < 0)
-		xabort("xlseek");
+	xabort_on_syserror(result, __func__);
 
 	return result;
 }
 
 void
 xread(long fd, void *buffer, size_t size)
 {
-	if (syscall_no_intercept(SYS_read, fd,
-	    buffer, size) != (ssize_t)size)
-		xabort("xread");
+	long result = syscall_no_intercept(SYS_read, fd, buffer, size);
+
+	if (result != (long)size)
+		xabort_errno(syscall_error_code(result), __func__);
 }
 
 /*
@@ -132,8 +132,7 @@ intercept_setup_log(const char *path_base, const char *trunc)
 
 	log_fd = (int)syscall_no_intercept(SYS_open, path, flags, (mode_t)0700);
 
-	if (log_fd < 0)
-		xabort("setup_log");
+	xabort_on_syserror(log_fd, "opening log");
 }
 
 /*

src/patcher.c

@@ -808,6 +808,15 @@ after_nop(const struct range *nop)
 	return nop->address + nop->size;
 }
 
+static void
+mprotect_no_intercept(void *addr, size_t len, int prot,
+			const char *msg_on_error)
+{
+	long result = syscall_no_intercept(SYS_mprotect, addr, len, prot);
+
+	xabort_on_syserror(result, msg_on_error);
+}
+
 /*
  * activate_patches()
  * Loop over all the patches, and and overwrite each syscall.
@@ -824,9 +833,9 @@ activate_patches(struct intercept_desc *desc)
 	first_page = round_down_address(desc->text_start);
 	size = (size_t)(desc->text_end - first_page);
 
-	if (syscall_no_intercept(SYS_mprotect, first_page, size,
-	    PROT_READ | PROT_WRITE | PROT_EXEC) != 0)
-		xabort("mprotect PROT_READ | PROT_WRITE | PROT_EXEC");
+	mprotect_no_intercept(first_page, size,
+	    PROT_READ | PROT_WRITE | PROT_EXEC,
+	    "mprotect PROT_READ | PROT_WRITE | PROT_EXEC");
 
 	for (unsigned i = 0; i < desc->count; ++i) {
 		const struct patch_desc *patch = desc->items + i;
@@ -902,9 +911,9 @@ activate_patches(struct intercept_desc *desc)
 		}
 	}
 
-	if (syscall_no_intercept(SYS_mprotect, first_page, size,
-	    PROT_READ | PROT_EXEC) != 0)
-		xabort("mprotect PROT_READ | PROT_EXEC");
+	mprotect_no_intercept(first_page, size,
+	    PROT_READ | PROT_EXEC,
+	    "mprotect PROT_READ | PROT_EXEC");
 }
 
 /*
@@ -941,9 +950,9 @@ next_asm_wrapper_space(void)
 void
 mprotect_asm_wrappers(void)
 {
-	if (syscall_no_intercept(SYS_mprotect,
+	mprotect_no_intercept(
 	    round_down_address(asm_wrapper_space + PAGE_SIZE),
 	    sizeof(asm_wrapper_space) - PAGE_SIZE,
-	    PROT_READ | PROT_EXEC) != 0)
-		xabort("mprotect_asm_wrappers PROT_READ | PROT_EXEC");
+	    PROT_READ | PROT_EXEC,
+	    "mprotect_asm_wrappers PROT_READ | PROT_EXEC");
 }