[Tinycc-devel] [PATCH] libtcc: tcc_relocate(): avoid excessive permissions on caller-allocated memory zones

[Yichun Zhang (agentzh)]

Add new libtcc C API function, tcc_set_exec_section_align(alignment),
to ensure page-aligned executable sections (successive executable
sections can share the same page). Defaults to RUN_SECTION_ALIGNMENT as
before.

Also always call mprotect() even in selinux mode to avoid setting
executable permissions on the whole ELF image allocated by the caller.

Added corresponding regression tests to cover the new feature and
changes.
---
 libtcc.c                                           |  11 ++
 libtcc.h                                           |   2 +
 tcc.h                                              |   7 ++
 tccrun.c                                           |  59 ++++++----
 tests/tests2/99_set_exec_section_align_0.c         | 121 ++++++++++++++++++++
 tests/tests2/99_set_exec_section_align_0.expect    |   4 +
 tests/tests2/99_set_exec_section_align_4096.c      | 122 +++++++++++++++++++++
 tests/tests2/99_set_exec_section_align_4096.expect |   4 +
 8 files changed, 309 insertions(+), 21 deletions(-)
 create mode 100644 tests/tests2/99_set_exec_section_align_0.c
 create mode 100644 tests/tests2/99_set_exec_section_align_0.expect
 create mode 100644 tests/tests2/99_set_exec_section_align_4096.c
 create mode 100644 tests/tests2/99_set_exec_section_align_4096.expect

diff --git a/libtcc.c b/libtcc.c
index 1e9dd97..16a7906 100644
--- a/libtcc.c
+++ b/libtcc.c
@@ -733,6 +733,7 @@ LIBTCCAPI TCCState *tcc_new(void)
     tcc_state = s;
     ++nb_states;
 
+    s->exec_section_align = RUN_SECTION_ALIGNMENT;
     s->alacarte_link = 1;
     s->nocommon = 1;
     s->warn_implicit_function_declaration = 1;
@@ -1979,3 +1980,13 @@ PUB_FUNC void tcc_print_stats(TCCState *s, unsigned 
total_time)
     fprintf(stderr, "* %d bytes memory used\n", mem_max_size);
 #endif
 }
+
+LIBTCCAPI void tcc_set_exec_section_align(TCCState *s, unsigned alignment)
+{
+    if (alignment <= RUN_SECTION_ALIGNMENT + 1)
+        alignment = RUN_SECTION_ALIGNMENT;
+    else
+        alignment--;
+
+    s->exec_section_align = alignment;
+}
diff --git a/libtcc.h b/libtcc.h
index a1b31e3..e7b3dbf 100644
--- a/libtcc.h
+++ b/libtcc.h
@@ -19,6 +19,8 @@ LIBTCCAPI TCCState *tcc_new(void);
 /* free a TCC compilation context */
 LIBTCCAPI void tcc_delete(TCCState *s);
 
+LIBTCCAPI void tcc_set_exec_section_align(TCCState *s, unsigned alignment);
+
 /* set CONFIG_TCCDIR at runtime */
 LIBTCCAPI void tcc_set_lib_path(TCCState *s, const char *path);
 
diff --git a/tcc.h b/tcc.h
index cd67973..6ce425c 100644
--- a/tcc.h
+++ b/tcc.h
@@ -109,6 +109,12 @@ extern long double strtold (const char *__nptr, char 
**__endptr);
 # define PATHSEP ":"
 #endif
 
+#if defined TCC_TARGET_I386 || defined TCC_TARGET_X86_64
+ #define RUN_SECTION_ALIGNMENT 63
+#else
+ #define RUN_SECTION_ALIGNMENT 15
+#endif
+
 /* -------------------------------------------- */
 
 /* parser debug */
@@ -639,6 +645,7 @@ struct sym_attr {
 
 struct TCCState {
 
+    unsigned exec_section_align;  /* set executable section alignment */
     int verbose; /* if true, display some information during compilation */
     int nostdinc; /* if true, no standard headers are added */
     int nostdlib; /* if true, no standard libraries are added */
diff --git a/tccrun.c b/tccrun.c
index 9360164..e1aadbb 100644
--- a/tccrun.c
+++ b/tccrun.c
@@ -36,6 +36,9 @@
 # else
 #  define ucontext_t CONTEXT
 # endif
+
+#define TCC_ALIGN(d, a)     (((d) + a) & ~a)
+
 ST_DATA int rt_num_callers = 6;
 ST_DATA const char **rt_bound_error_msg;
 ST_DATA void *rt_prog_main;
@@ -44,7 +47,7 @@ static void rt_error(ucontext_t *uc, const char *fmt, ...);
 static void set_exception_handler(void);
 #endif
 
-static void set_pages_executable(void *ptr, unsigned long length);
+static void *set_pages_executable(void *ptr, void *prev, unsigned long length);
 static int tcc_relocate_ex(TCCState *s1, void *ptr, addr_t ptr_diff);
 
 #ifdef _WIN64
@@ -168,12 +171,6 @@ LIBTCCAPI int tcc_run(TCCState *s1, int argc, char **argv)
     return (*prog_main)(argc, argv);
 }
 
-#if defined TCC_TARGET_I386 || defined TCC_TARGET_X86_64
- #define RUN_SECTION_ALIGNMENT 63
-#else
- #define RUN_SECTION_ALIGNMENT 15
-#endif
-
 /* relocate code. Return -1 on error, required size if ptr is NULL,
    otherwise copy code into buffer passed by the caller */
 static int tcc_relocate_ex(TCCState *s1, void *ptr, addr_t ptr_diff)
@@ -181,6 +178,8 @@ static int tcc_relocate_ex(TCCState *s1, void *ptr, addr_t 
ptr_diff)
     Section *s;
     unsigned offset, length, fill, i, k;
     addr_t mem;
+    void *prev;
+    unsigned alignment;
 
     if (NULL == ptr) {
         s1->nb_errors = 0;
@@ -196,7 +195,8 @@ static int tcc_relocate_ex(TCCState *s1, void *ptr, addr_t 
ptr_diff)
     }
 
     offset = 0, mem = (addr_t)ptr;
-    fill = -mem & RUN_SECTION_ALIGNMENT;
+    alignment = s1->exec_section_align;
+    fill = -mem & alignment;
 #ifdef _WIN64
     offset += sizeof (void*);
 #endif
@@ -222,12 +222,12 @@ static int tcc_relocate_ex(TCCState *s1, void *ptr, 
addr_t ptr_diff)
             offset += s->data_offset;
             fill = -(mem + offset) & 15;
         }
-#if RUN_SECTION_ALIGNMENT > 15
-        /* To avoid that x86 processors would reload cached instructions each 
time
-           when data is written in the near, we need to make sure that code 
and data
-           do not share the same 64 byte unit */
-        fill = -(mem + offset) & RUN_SECTION_ALIGNMENT;
-#endif
+        if (alignment > 15) {
+            /* To avoid that x86 processors would reload cached instructions 
each time
+               when data is written in the near, we need to make sure that 
code and data
+               do not share the same 64 byte unit (or a custom alignment) */
+            fill = -(mem + offset) & alignment;
+        }
     }
 
     /* relocate symbols */
@@ -236,7 +236,7 @@ static int tcc_relocate_ex(TCCState *s1, void *ptr, addr_t 
ptr_diff)
         return -1;
 
     if (0 == mem)
-        return offset + RUN_SECTION_ALIGNMENT;
+        return TCC_ALIGN(offset, alignment);
 
 #ifdef TCC_TARGET_PE
     s1->pe_imagebase = mem;
@@ -262,9 +262,19 @@ static int tcc_relocate_ex(TCCState *s1, void *ptr, addr_t 
ptr_diff)
             memset(ptr, 0, length);
         else
             memcpy(ptr, s->data, length);
+    }
+
+    prev = NULL;
+    for(i = 1; i < s1->nb_sections; i++) {
+        s = s1->sections[i];
+        if (0 == (s->sh_flags & SHF_ALLOC))
+            continue;
+        if (!(s->sh_flags & SHF_EXECINSTR))
+            continue;
+        ptr = (void *) s->sh_addr;
+        length = s->data_offset;
         /* mark executable sections as executable in memory */
-        if (s->sh_flags & SHF_EXECINSTR)
-            set_pages_executable((char*)ptr + ptr_diff, length);
+        prev = set_pages_executable(ptr, prev, TCC_ALIGN(length, alignment));
     }
 
 #ifdef _WIN64
@@ -277,27 +287,34 @@ static int tcc_relocate_ex(TCCState *s1, void *ptr, 
addr_t ptr_diff)
 /* ------------------------------------------------------------- */
 /* allow to run code in memory */
 
-static void set_pages_executable(void *ptr, unsigned long length)
+static void *set_pages_executable(void *ptr, void *prev, unsigned long length)
 {
 #ifdef _WIN32
     unsigned long old_protect;
+    if (prev == ptr)
+        return ptr;
     VirtualProtect(ptr, length, PAGE_EXECUTE_READWRITE, &old_protect);
+    return ptr;
 #else
     void __clear_cache(void *beginning, void *end);
-# ifndef HAVE_SELINUX
     addr_t start, end;
 #  ifndef PAGESIZE
 #   define PAGESIZE 4096
 #  endif
     start = (addr_t)ptr & ~(PAGESIZE - 1);
+    if ((void *) start == prev)
+        return prev;
     end = (addr_t)ptr + length;
     end = (end + PAGESIZE - 1) & ~(PAGESIZE - 1);
-    if (mprotect((void *)start, end - start, PROT_READ | PROT_WRITE | 
PROT_EXEC))
+#if 0
+    printf("mprotect %p %ld\n", (void *) start, end - start);
+#endif
+    if (mprotect((void *)start, end - start, PROT_READ | PROT_EXEC))
         tcc_error("mprotect failed: did you mean to configure 
--with-selinux?");
-# endif
 # if defined TCC_TARGET_ARM || defined TCC_TARGET_ARM64
     __clear_cache(ptr, (char *)ptr + length);
 # endif
+    return (void *) start;
 #endif
 }
 
diff --git a/tests/tests2/99_set_exec_section_align_0.c 
b/tests/tests2/99_set_exec_section_align_0.c
new file mode 100644
index 0000000..67cccd1
--- /dev/null
+++ b/tests/tests2/99_set_exec_section_align_0.c
@@ -0,0 +1,121 @@
+/*
+ * Simple Test program for libtcc
+ *
+ * libtcc can be useful to use tcc as a "backend" for a code generator.
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stddef.h>
+#include <sys/mman.h>
+
+#include "libtcc.h"
+
+/* this function is called by the generated code */
+int add(int a, int b)
+{
+    return a + b;
+}
+
+char my_program[] =
+"#include <tcclib.h>\n" /* include the "Simple libc header for TCC" */
+"int a, b;\n"
+"int c, d;\n"
+"#ifdef _WIN32\n" /* dynamically linked data needs 'dllimport' */
+" __attribute__((dllimport))\n"
+"#endif\n"
+"int fib(int n)\n"
+"{\n"
+"    printf(\"a = %d, b = %d, c = %d, d = %d\\n\", a, b, c, d);\n"
+"    if (n <= 2)\n"
+"        return 1;\n"
+"    else\n"
+"        return fib(n-1) + fib(n-2);\n"
+"}\n"
+"\n"
+"int foo(int n)\n"
+"{\n"
+"    printf(\"fib(%d) = %d\\n\", n, fib(n));\n"
+"    return 0;\n"
+"}\n";
+
+int main(int argc, char **argv)
+{
+    TCCState *s;
+    int i;
+    int (*func)(int);
+    static int e = 15;
+
+    s = tcc_new();
+    if (!s) {
+        fprintf(stderr, "Could not create tcc state\n");
+        exit(1);
+    }
+
+    tcc_set_options(s, "-g");
+    tcc_set_exec_section_align(s, 0);
+
+    /* if tcclib.h and libtcc1.a are not installed, where can we find them */
+    for (i = 1; i < argc; ++i) {
+        char *a = argv[i];
+        if (a[0] == '-') {
+            if (a[1] == 'B')
+                tcc_set_lib_path(s, a+2);
+            else if (a[1] == 'I')
+                tcc_add_include_path(s, a+2);
+            else if (a[1] == 'L')
+                tcc_add_library_path(s, a+2);
+        }
+    }
+
+    /* MUST BE CALLED before any compilation */
+    tcc_set_output_type(s, TCC_OUTPUT_MEMORY);
+
+    if (tcc_compile_string(s, my_program) == -1) {
+        return 1;
+    }
+
+    /* relocate the code */
+    {
+        size_t len;
+        char *p, *q;
+
+        len = tcc_relocate(s, NULL);
+        if (len < 0) {
+            printf("tcc_relocate failed\n");
+            return 1;
+        }
+
+        p = mmap (NULL, len, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, 0, 0);
+        if (p == MAP_FAILED) {
+            printf("mmap failed\n");
+            return 1;
+        }
+
+        if (tcc_relocate(s, p) < 0) {
+            printf("tcc_relocate failed\n");
+            return 1;
+        }
+
+        //printf("p = %p, len = %d\n", p, len);
+    }
+
+    /* get entry symbol */
+    func = tcc_get_symbol(s, "foo");
+    if (!func) {
+        printf("tcc_get_symbol failed\n");
+        return 1;
+    }
+
+    //printf("func addr: %p\n", func);
+
+    /* delete the state */
+    tcc_delete(s);
+
+    /* run the code */
+    func(3);
+
+    //fprintf(stderr, "exiting...");
+
+    return 0;
+}
diff --git a/tests/tests2/99_set_exec_section_align_0.expect 
b/tests/tests2/99_set_exec_section_align_0.expect
new file mode 100644
index 0000000..e727f6e
--- /dev/null
+++ b/tests/tests2/99_set_exec_section_align_0.expect
@@ -0,0 +1,4 @@
+a = 0, b = 0, c = 0, d = 0
+a = 0, b = 0, c = 0, d = 0
+a = 0, b = 0, c = 0, d = 0
+fib(3) = 2
diff --git a/tests/tests2/99_set_exec_section_align_4096.c 
b/tests/tests2/99_set_exec_section_align_4096.c
new file mode 100644
index 0000000..838b143
--- /dev/null
+++ b/tests/tests2/99_set_exec_section_align_4096.c
@@ -0,0 +1,122 @@
+/*
+ * Simple Test program for libtcc
+ *
+ * libtcc can be useful to use tcc as a "backend" for a code generator.
+ */
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stddef.h>
+#include <sys/mman.h>
+
+#include "libtcc.h"
+
+/* this function is called by the generated code */
+int add(int a, int b)
+{
+    return a + b;
+}
+
+char my_program[] =
+"#include <tcclib.h>\n" /* include the "Simple libc header for TCC" */
+"int a, b;\n"
+"int c, d;\n"
+"#ifdef _WIN32\n" /* dynamically linked data needs 'dllimport' */
+" __attribute__((dllimport))\n"
+"#endif\n"
+"int fib(int n)\n"
+"{\n"
+"    a++; b++; c++; d++;\n"
+"    printf(\"a = %d, b = %d, c = %d, d = %d\\n\", a, b, c, d);\n"
+"    if (n <= 2)\n"
+"        return 1;\n"
+"    else\n"
+"        return fib(n-1) + fib(n-2);\n"
+"}\n"
+"\n"
+"int foo(int n)\n"
+"{\n"
+"    printf(\"fib(%d) = %d\\n\", n, fib(n));\n"
+"    return 0;\n"
+"}\n";
+
+int main(int argc, char **argv)
+{
+    TCCState *s;
+    int i;
+    int (*func)(int);
+    static int e = 15;
+
+    s = tcc_new();
+    if (!s) {
+        fprintf(stderr, "Could not create tcc state\n");
+        exit(1);
+    }
+
+    tcc_set_options(s, "-g");
+    tcc_set_exec_section_align(s, 4096);
+
+    /* if tcclib.h and libtcc1.a are not installed, where can we find them */
+    for (i = 1; i < argc; ++i) {
+        char *a = argv[i];
+        if (a[0] == '-') {
+            if (a[1] == 'B')
+                tcc_set_lib_path(s, a+2);
+            else if (a[1] == 'I')
+                tcc_add_include_path(s, a+2);
+            else if (a[1] == 'L')
+                tcc_add_library_path(s, a+2);
+        }
+    }
+
+    /* MUST BE CALLED before any compilation */
+    tcc_set_output_type(s, TCC_OUTPUT_MEMORY);
+
+    if (tcc_compile_string(s, my_program) == -1) {
+        return 1;
+    }
+
+    /* relocate the code */
+    {
+        size_t len;
+        char *p, *q;
+
+        len = tcc_relocate(s, NULL);
+        if (len < 0) {
+            printf("tcc_relocate failed\n");
+            return 1;
+        }
+
+        p = mmap (NULL, len, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, 0, 0);
+        if (p == MAP_FAILED) {
+            printf("mmap failed\n");
+            return 1;
+        }
+
+        if (tcc_relocate(s, p) < 0) {
+            printf("tcc_relocate failed\n");
+            return 1;
+        }
+
+        //printf("p = %p, len = %d\n", p, len);
+    }
+
+    /* get entry symbol */
+    func = tcc_get_symbol(s, "foo");
+    if (!func) {
+        printf("tcc_get_symbol failed\n");
+        return 1;
+    }
+
+    //printf("func addr: %p\n", func);
+
+    /* delete the state */
+    tcc_delete(s);
+
+    /* run the code */
+    func(3);
+
+    //fprintf(stderr, "exiting...");
+
+    return 0;
+}
diff --git a/tests/tests2/99_set_exec_section_align_4096.expect 
b/tests/tests2/99_set_exec_section_align_4096.expect
new file mode 100644
index 0000000..e6b9a36
--- /dev/null
+++ b/tests/tests2/99_set_exec_section_align_4096.expect
@@ -0,0 +1,4 @@
+a = 1, b = 1, c = 1, d = 1
+a = 2, b = 2, c = 2, d = 2
+a = 3, b = 3, c = 3, d = 3
+fib(3) = 2
-- 
2.11.0.295.gd7dffce