compel: fix the stack test

The stack test incorrectly assumed the page immediately
following the stack pointer could never be changed. This doesn't work,
because this page can be a part of another mapping.

This commit introduces a dedicated "stack redzone," a small guard region
directly after the stack. The stack test is modified to specifically
check for corruption within this redzone.

Signed-off-by: Andrei Vagin <avagin@gmail.com>

compel/include/uapi/infect.h

@@ -13,6 +13,15 @@
 
 #define PARASITE_START_AREA_MIN (4096)
 
+#define PARASITE_STACK_SIZE (16 << 10)
+/*
+ * A stack redzone is a small, protected region of memory located immediately
+ * after a parasite stack. It is intended to remain unchanged. While it can be
+ * implemented as a guard page, we want to avoid the overhead of additional
+ * remote system calls.
+ */
+#define PARASITE_STACK_REDZONE 128
+
 extern int __must_check compel_interrupt_task(int pid);
 
 struct seize_task_status {

compel/src/lib/infect.c

@@ -38,8 +38,6 @@
 #define UNIX_PATH_MAX (sizeof(struct sockaddr_un) - (size_t)((struct sockaddr_un *)0)->sun_path)
 #endif
 
-#define PARASITE_STACK_SIZE (16 << 10)
-
 #ifndef SECCOMP_MODE_DISABLED
 #define SECCOMP_MODE_DISABLED 0
 #endif
@@ -1064,7 +1062,7 @@ int compel_infect_no_daemon(struct parasite_ctl *ctl, unsigned long nr_threads,
 
 	p += RESTORE_STACK_SIGFRAME;
 	p += PARASITE_STACK_SIZE;
-	ctl->rstack = ctl->remote_map + p;
+	ctl->rstack = ctl->remote_map + p - PARASITE_STACK_REDZONE;
 
 	/*
 	 * x86-64 ABI requires a 16 bytes aligned stack.
@@ -1078,7 +1076,7 @@ int compel_infect_no_daemon(struct parasite_ctl *ctl, unsigned long nr_threads,
 
 	if (nr_threads > 1) {
 		p += PARASITE_STACK_SIZE;
-		ctl->r_thread_stack = ctl->remote_map + p;
+		ctl->r_thread_stack = ctl->remote_map + p - PARASITE_STACK_REDZONE;
 	}
 
 	ret = arch_fetch_sas(ctl, ctl->rsigframe);

compel/test/stack/spy.c

@@ -50,70 +50,6 @@ static void *get_parasite_rstack_start(struct parasite_ctl *ctl)
 	return rstack_start;
 }
 
-static int page_writable(struct parasite_ctl *ctl, int pid, void *page)
-{
-	FILE *maps;
-	size_t maps_line_len = 0;
-	char *maps_line = NULL;
-	char victim_maps_path[6 + 11 + 5 + 1];
-	int written;
-	int ret = 0;
-
-	if (((uintptr_t)page & (page_size() - 1)) != 0) {
-		fprintf(stderr, "Page address not aligned\n");
-		ret = -1;
-		goto done;
-	}
-
-	written = snprintf(victim_maps_path, sizeof(victim_maps_path), "/proc/%d/maps", pid);
-	if (written < 0 || written >= sizeof(victim_maps_path)) {
-		fprintf(stderr, "Failed to create path string to victim's /proc/%d/maps file\n", pid);
-		ret = -1;
-		goto done;
-	}
-
-	maps = fopen(victim_maps_path, "r");
-	if (maps == NULL) {
-		perror("Can't open victim's /proc/$pid/maps");
-		ret = -1;
-		goto done;
-	}
-
-	while (getline(&maps_line, &maps_line_len, maps) != -1) {
-		unsigned long vmstart, vmend;
-		char r, w;
-
-		if (sscanf(maps_line, "%lx-%lx %c%c", &vmstart, &vmend, &r, &w) < 4) {
-			fprintf(stderr, "Can't parse victim's /proc/%d/maps; line: %s\n", pid, maps_line);
-			ret = -1;
-			goto free_linebuf;
-		}
-
-		if (page >= (void *)vmstart && page < (void *)vmend) {
-			if (w == 'w') {
-				if (r != 'r') {
-					fprintf(stderr, "Expecting writable memory to also be readable");
-					ret = -1;
-					goto free_linebuf;
-				}
-				ret = 1;
-			}
-			break;
-		}
-	}
-
-	if (errno) {
-		perror("Can't read victim's /proc/$pid/maps");
-		ret = -1;
-	}
-
-free_linebuf:
-	free(maps_line);
-	fclose(maps);
-done:
-	return ret;
-}
-
 static void *read_proc_mem(int pid, void *offset, size_t len)
 {
 	char victim_mem_path[6 + 11 + 4 + 1];
@@ -153,51 +89,6 @@ freebuf:
 	return NULL;
 }
 
-static int save_data_near_stack(struct parasite_ctl *ctl, int pid, void *stack, void **saved_data,
-				size_t *saved_data_size)
-{
-	size_t page_mask = page_size() - 1;
-	size_t saved_size = 0;
-	size_t stack_size_last_page = (uintptr_t)stack & page_mask;
-	void *next_page = stack;
-
-	if (stack_size_last_page != 0) {
-		size_t empty_space_last_page = page_size() - stack_size_last_page;
-		saved_size = min(empty_space_last_page, (size_t)SAVED_DATA_MAX);
-		next_page += page_size() - stack_size_last_page;
-	}
-
-	while (saved_size < SAVED_DATA_MAX && next_page != NULL) {
-		switch (page_writable(ctl, pid, next_page)) {
-		case 1:
-			saved_size = min((size_t)(saved_size + page_size()), (size_t)SAVED_DATA_MAX);
-			next_page += page_size();
-			break;
-		case 0:
-			next_page = NULL;
-			break;
-		default:
-			return -1;
-		}
-	}
-
-	if (saved_size > 0) {
-		void *sd;
-
-		sd = read_proc_mem(pid, stack, saved_size);
-		if (sd == NULL)
-			return -1;
-
-		*saved_data = sd;
-	} else {
-		*saved_data = NULL;
-	}
-
-	*saved_data_size = saved_size;
-
-	return 0;
-}
-
 static int check_saved_data(struct parasite_ctl *ctl, int pid, void *stack, void *saved_data, size_t saved_data_size)
 {
 	if (saved_data != NULL) {
@@ -221,7 +112,7 @@ static int do_infection(int pid)
 	struct infect_ctx *ictx;
 	int *arg;
 	void *stack;
-	size_t saved_data_size;
+	size_t saved_data_size = PARASITE_STACK_REDZONE;
 	int saved_data_check;
 
 	compel_log_init(print_vmsg, COMPEL_LOG_DEBUG);
@@ -257,8 +148,6 @@ static int do_infection(int pid)
 		err_and_ret("Can't register cleanup function with atexit\n");
 
 	stack = get_parasite_rstack_start(ctl);
-	if (save_data_near_stack(ctl, pid, stack, &saved_data, &saved_data_size))
-		err_and_ret("Can't save data above stack\n");
 
 	if (compel_start_daemon(ctl))
 		err_and_ret("Can't start daemon in victim\n");