compel: allocate the GOT table to avoid memory corruption

Previously, the GOT table was using the same memory location as the
args region, leading to difficult to debug memory corruption bugs.

We allocate the GOT table between the parasite blob and the args region.
The reason this is a good placement is:
1) Putting it after the args region is possible but a bit combersome as
the args region has a variable size
2) The cr-restore.c code maps the parasite code without the args region,
as it does not do RPC.

Another option is to rely on the linker to generate a GOT section, but I
failed to do so despite my best attempts.

Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com>

compel/include/uapi/infect.h

@@ -150,10 +150,10 @@ struct parasite_blob_desc {
 		struct {
 			const void		*mem;
 			size_t			bsize;
-			size_t			nr_gotpcrel;
 			unsigned long		parasite_ip_off;
 			unsigned long		cmd_off;
 			unsigned long		args_ptr_off;
+			unsigned long		got_off;
 			unsigned long		args_off;
 			compel_reloc_t		*relocs;
 			unsigned int		nr_relocs;

compel/src/lib/handle-elf.c

@@ -648,7 +648,6 @@ int __handle_elf(void *mem, size_t size)
 	}
 #endif /* !NO_RELOCS */
 	pr_out("};\n");
-	pr_out("static __maybe_unused size_t %s_nr_gotpcrel = %zd;\n", opts.prefix, nr_gotpcrel);
 
 	pr_out("static __maybe_unused const char %s_blob[] = {\n\t", opts.prefix);
 
@@ -690,7 +689,6 @@ int __handle_elf(void *mem, size_t size)
 	pr_out("\tpbd->hdr.mem		= %s_blob;\n", opts.prefix);
 	pr_out("\tpbd->hdr.bsize		= sizeof(%s_blob);\n",
 			opts.prefix);
-	pr_out("\tpbd->hdr.nr_gotpcrel	= %s_nr_gotpcrel;\n", opts.prefix);
 	pr_out("\tif (native)\n");
 	pr_out("\t\tpbd->hdr.parasite_ip_off	= "
 		"%s_sym__export_parasite_head_start;\n", opts.prefix);
@@ -701,7 +699,8 @@ int __handle_elf(void *mem, size_t size)
 	pr_out("#endif /* CONFIG_COMPAT */\n");
 	pr_out("\tpbd->hdr.cmd_off	= %s_sym__export_parasite_service_cmd;\n", opts.prefix);
 	pr_out("\tpbd->hdr.args_ptr_off	= %s_sym__export_parasite_service_args_ptr;\n", opts.prefix);
-	pr_out("\tpbd->hdr.args_off	= round_up(pbd->hdr.bsize, 4);\n");
+	pr_out("\tpbd->hdr.got_off	= round_up(pbd->hdr.bsize, sizeof(long));\n");
+	pr_out("\tpbd->hdr.args_off	= pbd->hdr.got_off + %zd*sizeof(long);\n", nr_gotpcrel);
 	pr_out("\tpbd->hdr.relocs		= %s_relocs;\n", opts.prefix);
 	pr_out("\tpbd->hdr.nr_relocs	= "
 			"sizeof(%s_relocs) / sizeof(%s_relocs[0]);\n",

compel/src/lib/infect.c

@@ -816,11 +816,11 @@ err_cure:
 
 void compel_relocs_apply(void *mem, void *vbase, struct parasite_blob_desc *pbd)
 {
-	size_t size = pbd->hdr.bsize;
 	compel_reloc_t *elf_relocs = pbd->hdr.relocs;
 	size_t nr_relocs = pbd->hdr.nr_relocs;
 
 	size_t i, j;
+	void **got = mem + pbd->hdr.got_off;
 
 	/*
 	 * parasite_service() reads the value of __export_parasite_service_args_ptr.
@@ -835,14 +835,13 @@ void compel_relocs_apply(void *mem, void *vbase, struct parasite_blob_desc *pbd)
 	for (i = 0, j = 0; i < nr_relocs; i++) {
 		if (elf_relocs[i].type & COMPEL_TYPE_LONG) {
 			long *where = mem + elf_relocs[i].offset;
-			long *p = mem + size;
 
 			if (elf_relocs[i].type & COMPEL_TYPE_GOTPCREL) {
 				int *value = (int *)where;
 				int rel;
 
-				p[j] = (long)vbase + elf_relocs[i].value;
+				got[j] = vbase + elf_relocs[i].value;
-				rel = (unsigned)((void *)&p[j] - (void *)mem) - elf_relocs[i].offset + elf_relocs[i].addend;
+				rel = (unsigned)((void *)&got[j] - (void *)mem) - elf_relocs[i].offset + elf_relocs[i].addend;
 
 				*value = rel;
 				j++;
@@ -899,7 +898,9 @@ int compel_infect(struct parasite_ctl *ctl, unsigned long nr_threads, unsigned l
 	 * +------------------------------------------------------+ <--- 0
 	 * |   Parasite blob (sizeof(parasite_blob))              |
 	 * +------------------------------------------------------+ <--- hdr.bsize
-	 *                         align 4
+	 *                         align 8
+	 * +------------------------------------------------------+ <--- hdr.got_off
+	 * |   GOT Table (nr_gotpcrel * sizeof(long))             |
 	 * +------------------------------------------------------+ <--- hdr.args_off
 	 * |   Args area (args_size)                              |
 	 * +------------------------------------------------------+
@@ -936,6 +937,16 @@ int compel_infect(struct parasite_ctl *ctl, unsigned long nr_threads, unsigned l
 	ctl->cmd = ctl->local_map + ctl->pblob.hdr.cmd_off;
 	ctl->args = ctl->local_map + ctl->pblob.hdr.args_off;
 
+	/*
+	 * args must be 4 bytes aligned as we use futexes() on them. It is
+	 * already the case, as args follows the GOT table, which is 8 bytes
+	 * aligned.
+	 */
+	if ((unsigned long)ctl->args & (4-1)) {
+		pr_err("BUG: args are not 4 bytes aligned: %p\n", ctl->args);
+		ctl->args = (void *)round_up((unsigned long)ctl->args, 4);
+	}
+
 	memcpy(ctl->local_map, ctl->pblob.hdr.mem, ctl->pblob.hdr.bsize);
 	compel_relocs_apply(ctl->local_map, ctl->remote_map, &ctl->pblob);
 

criu/cr-restore.c

@@ -96,8 +96,6 @@
 
 #include "cr-errno.h"
 
-#include "pie/pie-relocs.h"
-
 #ifndef arch_export_restore_thread
 #define arch_export_restore_thread	__export_restore_thread
 #endif
@@ -2913,7 +2911,23 @@ static int prepare_restorer_blob(void)
 	 * in turn will lead to set-exe-file prctl to fail with EBUSY.
 	 */
 
-	restorer_len = pie_size(restorer);
+	struct parasite_blob_desc pbd;
+
+	/*
+	 * We pass native=true, which is then used to set the value of
+	 * pbd.parasite_ip_off. We don't use parasite_ip_off, so the value we
+	 * pass as native argument is not relevant.
+	 */
+	restorer_setup_c_header_desc(&pbd, true);
+
+	/*
+	 * args_off is the offset where the binary blob with its GOT table
+	 * ends. As we don't do RPC, parasite sections after args_off can be
+	 * ignored. See compel_infect() for a description of the parasite
+	 * memory layout.
+	 */
+	restorer_len = round_up(pbd.hdr.args_off, page_size());
+
 	restorer = mmap(NULL, restorer_len,
 			PROT_READ | PROT_WRITE | PROT_EXEC,
 			MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
@@ -2922,7 +2936,8 @@ static int prepare_restorer_blob(void)
 		return -1;
 	}
 
-	memcpy(restorer, &restorer_blob, sizeof(restorer_blob));
+	memcpy(restorer, pbd.hdr.mem, pbd.hdr.bsize);
+
 	return 0;
 }
 

criu/parasite-syscall.c

@@ -39,7 +39,6 @@
 
 #include "dump.h"
 #include "restorer.h"
-#include "pie/pie-relocs.h"
 
 #include "infect.h"
 #include "infect-rpc.h"

criu/pie/pie-relocs.h

@@ -1,10 +0,0 @@
-#ifndef __PIE_RELOCS_H__
-#define __PIE_RELOCS_H__
-
-#include "common/config.h"
-#include "common/compiler.h"
-
-#define pie_size(__pie_name)	(round_up(sizeof(__pie_name##_blob) + \
-			__pie_name ## _nr_gotpcrel * sizeof(long), page_size()))
-
-#endif /* __PIE_RELOCS_H__ */