Funchook install fail 1293

os/linux/Makefile

@@ -109,6 +109,7 @@ libtest: $(LIBRARY_C_FILES) $(LIBRARY_TEST_C_FILES) $(YAML_AR) $(JSON_AR) $(TEST
 	$(CC) $(TEST_CFLAGS) -o test/$(OS)/coredumptest coredumptest.o coredump.o scopestdlib.o dbg.o utils.o fn.o plattime.o os.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
 	$(CC) $(TEST_CFLAGS) -o test/$(OS)/ipctest ipctest.o ipc.o ipc_resp.o cfgutils.o cfg.o mtc.o log.o evtformat.o ctl.o transport.o backoff.o mtcformat.o strset.o com.o scopestdlib.o dbg.o circbuf.o linklist.o fn.o utils.o os.o test.o report.o search.o httpagg.o state.o httpstate.o metriccapture.o plattime.o $(TEST_AR) $(TEST_LD_FLAGS) -Wl,--wrap=jsonConfigurationObject -Wl,--wrap=doAndReplaceConfig
 	$(CC) $(TEST_CFLAGS) -o test/$(OS)/snapshottest snapshottest.o snapshot.o coredump.o scopestdlib.o dbg.o utils.o fn.o plattime.o os.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
+	$(CC) $(TEST_CFLAGS) -o test/$(OS)/ostest ostest.o scopestdlib.o dbg.o fn.o utils.o plattime.o os.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
 	$(CC) $(TEST_CFLAGS) -o test/$(OS)/strsettest strsettest.o strset.o scopestdlib.o dbg.o test.o $(TEST_AR) $(TEST_LD_FLAGS)
 	$(CC) $(TEST_CFLAGS) -o test/$(OS)/cfgutilstest cfgutilstest.o cfgutils.o cfg.o mtc.o log.o evtformat.o ctl.o transport.o backoff.o mtcformat.o strset.o com.o scopestdlib.o dbg.o circbuf.o linklist.o fn.o utils.o os.o test.o report.o search.o httpagg.o state.o httpstate.o metriccapture.o plattime.o $(TEST_AR) $(TEST_LD_FLAGS)
 	$(CC) $(TEST_CFLAGS) -o test/$(OS)/cfgtest cfgtest.o cfg.o scopestdlib.o dbg.o test.o $(TEST_AR) $(TEST_LD_FLAGS)

os/linux/os.c

@@ -869,7 +869,7 @@ osCreateSM(proc_id_t *proc, unsigned long addr)
         return;
     }
 
-    // size is initally 0 and needs to be increased
+    // size is initially 0 and needs to be increased
     if (scope_ftruncate(proc->smfd, sizeof(export_sm_t)) == -1) {
         scope_close(proc->smfd);
         return;
@@ -950,3 +950,25 @@ osFindFd(pid_t pid, const char *fname)
     free(cwd);
     return fd;
 }
+
+/*
+ * Change protection for specified memory region using 
+ * specified combintation flags and extraflags.
+ * Returns TRUE in case of operation success, FALSE otherwise
+ * Note: in case of success the `osMemPermRestore` should be called later
+ * with flags argument
+ */
+bool
+osMemPermAllow(void *addr, size_t len, int flags, int extraflags) {
+    return scope_mprotect(addr, len, flags | extraflags) == 0;
+}
+
+/*
+ * Restore permission for specified memory region.
+ * Returns TRUE in case of operation success, FALSE otherwise
+ * Note: This function should be called in case of success of `osMemPermAllow`
+ */
+bool
+osMemPermRestore(void *addr, size_t len, int flags) {
+    return scope_mprotect(addr, len, flags) == 0;
+}

os/linux/os.h

@@ -68,5 +68,7 @@ extern long long osGetProcCPU(void);
 extern uint64_t osFindLibrary(const char *, pid_t, bool);
 extern int osFindFd(pid_t, const char *);
 extern void osCreateSM(proc_id_t *, unsigned long);
+extern bool osMemPermAllow(void *, size_t, int, int);
+extern bool osMemPermRestore(void *, size_t, int);
 
 #endif  //__OS_H__

os/macOS/os.c

@@ -193,3 +193,13 @@ long long
 osGetProcCPU(void) {
     return -1;
 }
+
+bool
+osMemPermAllow(void *addr, size_t len, int flags, int extraflags) {
+    return FALSE;
+}
+
+bool
+osMemPermRestore(void *addr, size_t len, int flags) {
+    return FALSE;
+}

os/macOS/os.h

@@ -48,3 +48,5 @@ extern int osNeedsConnect(int);
 extern const char *osGetUserName(unsigned);
 extern const char *osGetGroupName(unsigned);
 extern long long osGetProcCPU(void);
+extern bool osMemPermAllow(void *, size_t, int, int);
+extern bool osMemPermRestore(void *, size_t, int);

src/scopeelf.c

@@ -201,9 +201,9 @@ doGotcha(struct link_map *lm, got_list_t *hook, Elf64_Rela *rel, Elf64_Sym *sym,
 
             if (prot != -1) {
                 if ((prot & PROT_WRITE) == 0) {
-                    // mprotect if write perms are not set
-                    if (scope_mprotect((void *)saddr, (size_t)16, PROT_WRITE | prot) == -1) {
-                        scopeLog(CFG_LOG_DEBUG, "doGotcha: mprotect failed");
+                    // allow for write permission it write permission are not set
+                    if (osMemPermAllow((void *)saddr, 16, prot, PROT_WRITE) == FALSE) {
+                        scopeLog(CFG_LOG_DEBUG, "doGotcha: osMemPermAllow add write protection flag failed");
                         return -1;
                     }
                 }
@@ -239,8 +239,8 @@ doGotcha(struct link_map *lm, got_list_t *hook, Elf64_Rela *rel, Elf64_Sym *sym,
 
             if ((prot & PROT_WRITE) == 0) {
                 // if we didn't mod above leave prot settings as is
-                if (scope_mprotect((void *)saddr, (size_t)16, prot) == -1) {
-                    scopeLog(CFG_LOG_DEBUG, "doGotcha: mprotect failed");
+                if (osMemPermRestore((void *)saddr, 16, prot) == FALSE) {
+                    scopeLog(CFG_LOG_DEBUG, "doGotcha: osMemPermRestore remove write memory protection flags failed");
                     return -1;
                 }
             }

src/wrap.c

@@ -1541,6 +1541,22 @@ initHook(int attachedFlag, bool scopedFlag)
     if (should_we_patch || g_fn.__write_libc || g_fn.__write_pthread ||
         ((g_ismusl == FALSE) && g_fn.sendmmsg) ||
         ((g_ismusl == TRUE) && (g_fn.sendto || g_fn.recvfrom))) {
+
+        /*
+        * Check if we have proper permission to install hook function
+        * We need to have abilites to change permission of region memory
+        * using (PROT_WRITE + PROT_EXEC) flags
+        */
+        size_t testSize = 16;
+        void *ptr = scope_malloc(testSize);
+        if (osMemPermAllow(ptr, testSize, PROT_READ | PROT_WRITE, PROT_EXEC) == FALSE) {
+            scope_free(ptr);
+            scopeLogError("The system is not allowing processes related to DNS or console I/O to be scoped. Try setting MemoryDenyWriteExecute to false for the %s service.", g_proc.procname);
+            return;
+        }
+        scope_free(ptr);
+
+
         funchook = funchook_create();
 
         if (logLevel(g_log) <= CFG_LOG_TRACE) {
@@ -1597,8 +1613,9 @@ initHook(int attachedFlag, bool scopedFlag)
         // hook 'em
         rc = funchook_install(funchook, 0);
         if (rc != 0) {
-            scopeLogError("ERROR: failed to install SSL_read hook. (%s)\n",
+            scopeLogError("ERROR: failed to install funchook. (%s)\n",
                         funchook_error_message(funchook));
+            funchook_destroy(funchook);
             return;
         }
     }

src/wrap_go.c

@@ -811,16 +811,18 @@ patch_addrs(funchook_t *funchook,
 }
 
 static void
-patchClone()
+patchClone(void)
 {
     void *clone = dlsym(RTLD_DEFAULT, "__clone");
     if (clone) {
         size_t pageSize = scope_getpagesize();
         void *addr = (void *)((ptrdiff_t) clone & ~(pageSize - 1));
 
-        // set write perms on the page
-        if (scope_mprotect(addr, pageSize, PROT_WRITE | PROT_READ | PROT_EXEC)) {
-            scopeLogError("ERROR: patchCLone: mprotect failed\n");
+        const int perm = PROT_READ | PROT_EXEC;
+
+        // Add write permission on the page
+        if (osMemPermAllow(addr, pageSize, perm, PROT_WRITE) == FALSE) {
+            scopeLogError("The system is not allowing processes to be scoped. Try setting MemoryDenyWriteExecute to false for the Go service.");
             return;
         }
 
@@ -832,9 +834,9 @@ patchClone()
 
         scopeLog(CFG_LOG_DEBUG, "patchClone: CLONE PATCHED\n");
 
-        // restore perms to the page
-        if (scope_mprotect(addr, pageSize, PROT_READ | PROT_EXEC)) {
-            scopeLogError("ERROR: patchCLone: mprotect restore failed\n");
+        // restore original permission to the page
+        if (osMemPermRestore(addr, pageSize, perm) == FALSE) {
+            scopeLogError("ERROR: patchClone: osMemPermRestore failed\n");
             return;
         }
     }
@@ -938,6 +940,7 @@ initGoHook(elf_buf_t *ebuf)
     if (ehdr->e_type == ET_DYN && (scopeGetGoAppStateStatic() == FALSE)) {
         if (getBaseAddress(&base) != 0) {
             sysprint("ERROR: can't get the base address\n");
+            funchook_destroy(funchook);
             return; // don't install our hooks
         }
         Elf64_Shdr* textSec = getElfSection(ebuf->buf, ".text");
@@ -977,9 +980,11 @@ initGoHook(elf_buf_t *ebuf)
         } else {
             scopeLogWarn("%s was either compiled with a version of go older than go1.4, or symbols have been stripped.  AppScope can only instrument go1.%d or newer, and requires symbols if compiled with a version of go older than go1.13.  Continuing without AppScope.", ebuf->cmd, MIN_SUPPORTED_GO_VER);
         }
+        funchook_destroy(funchook);
         return; // don't install our hooks
     } else if (g_go_minor_ver > MAX_SUPPORTED_GO_VER) {
         scopeLogWarn("%s was compiled with go version `%s`. Versions newer than Go 1.%d are not yet supported. Continuing without AppScope.", ebuf->cmd, go_runtime_version, MAX_SUPPORTED_GO_VER);
+        funchook_destroy(funchook);
         return; // don't install our hooks
     } 
 
@@ -1021,6 +1026,7 @@ initGoHook(elf_buf_t *ebuf)
             g_go_schema = &go_17_schema_arm;
         } else {
             scopeLogWarn("Architecture not supported. Continuing without AppScope.");
+            funchook_destroy(funchook);
             return;
         }
     }
@@ -1077,7 +1083,7 @@ initGoHook(elf_buf_t *ebuf)
     if (rc != 0) {
         sysprint("ERROR: funchook_install failed.  (%s)\n",
                 funchook_error_message(funchook));
-        return;
+        funchook_destroy(funchook);
     }
 }
 

test/unit/execute.sh

@@ -68,6 +68,7 @@ run_test test/${OS}/vdsotest
 run_test test/${OS}/coredumptest
 run_test test/${OS}/ipctest
 run_test test/${OS}/snapshottest
+run_test test/${OS}/ostest
 run_test test/${OS}/strsettest
 run_test test/${OS}/cfgutilstest
 run_test test/${OS}/cfgtest

test/unit/library/ostest.c

@@ -0,0 +1,45 @@
+#define _GNU_SOURCE
+
+#include "os.h"
+#include "scopestdlib.h"
+#include "test.h"
+
+static void
+osWritePermSuccess(void **state) {
+    int perm = PROT_READ | PROT_EXEC;
+    size_t len = 4096;
+    void *addr = scope_mmap(NULL, len, perm, MAP_ANONYMOUS | MAP_SHARED, -1, 0);
+    assert_ptr_not_equal(addr, MAP_FAILED);
+    bool res = osMemPermAllow(addr, len, perm, PROT_WRITE);
+    assert_true(res);
+    res = osMemPermRestore(addr, len, perm);
+    assert_true(res);
+    scope_munmap(addr ,len);
+}
+
+static void
+osWritePermFailure(void **state) {
+    int perm = PROT_READ;
+    size_t len = 4096;
+
+    // Open file as read only
+    int fd = scope_open("/etc/passwd", O_RDONLY);
+    void *addr = scope_mmap(NULL, len, perm, MAP_SHARED, fd, 0);
+    assert_ptr_not_equal(addr, MAP_FAILED);
+    bool res = osMemPermAllow(addr, len, perm, PROT_WRITE);
+    assert_false(res);
+    scope_munmap(addr ,len);
+    scope_close(fd);
+}
+
+int
+main(int argc, char* argv[]) {
+    printf("running %s\n", argv[0]);
+
+    const struct CMUnitTest tests[] = {
+        cmocka_unit_test(osWritePermSuccess),
+        cmocka_unit_test(osWritePermFailure),
+        cmocka_unit_test(dbgHasNoUnexpectedFailures),
+    };
+    return cmocka_run_group_tests(tests, groupSetup, groupTeardown);
+}