From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Wed, 27 Jan 2016 22:37:15 +0000 (-0700)
Subject: Add support for garbage collecting info passed to the plugin before
X-Git-Tag: SUDO_1_8_16^2~37
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9b3ef072f9ecc931b6f9c044e862ff8bcd59f4ae;p=sudo

Add support for garbage collecting info passed to the plugin before
exit to appease address sanitizer's leak detector (and valgrind's
leak checker).  We can't free these sooner since the plugin may be
using the memory.  For plugin API 2.0 it should be make clear that
the plugin must make a copy of the data in the arrays passed in to
the plugin's open() function.  Only enabled if NO_LEAKS is defined.
---

diff --git a/src/sudo.c b/src/sudo.c
index 8f01cd2f5..dbb79dafc 100644
--- a/src/sudo.c
+++ b/src/sudo.c
@@ -87,6 +87,23 @@ int sudo_debug_instance = SUDO_DEBUG_INSTANCE_INITIALIZER;
 static struct command_details command_details;
 static int sudo_mode;
 
+struct sudo_gc_entry {
+    SLIST_ENTRY(sudo_gc_entry) entries;
+    enum sudo_gc_types {
+	GC_UNKNOWN,
+	GC_VECTOR,
+	GC_PTR
+    } type;
+    union {
+	char **vec;
+	void *ptr;
+    } u;
+};
+SLIST_HEAD(sudo_gc_list, sudo_gc_entry);
+#ifdef NO_LEAKS
+static struct sudo_gc_list sudo_gc_list = SLIST_HEAD_INITIALIZER(sudo_gc_list);
+#endif
+
 /*
  * Local functions
  */
@@ -96,6 +113,8 @@ static void sudo_check_suid(const char *path);
 static char **get_user_info(struct user_details *);
 static void command_info_to_details(char * const info[],
     struct command_details *details);
+static bool gc_add(enum sudo_gc_types type, void *ptr);
+static void gc_exit(int exit_code) __attribute__((__noreturn__));
 
 /* Policy plugin convenience functions. */
 static int policy_open(struct plugin_container *plugin,
@@ -226,11 +245,11 @@ main(int argc, char *argv[], char *envp[])
 	case MODE_VALIDATE:
 	case MODE_VALIDATE|MODE_INVALIDATE:
 	    ok = policy_validate(&policy_plugin);
-	    exit(ok != 1);
+	    gc_exit(ok != 1);
 	case MODE_KILL:
 	case MODE_INVALIDATE:
 	    policy_invalidate(&policy_plugin, sudo_mode == MODE_KILL);
-	    exit(0);
+	    gc_exit(0);
 	    break;
 	case MODE_CHECK:
 	case MODE_CHECK|MODE_INVALIDATE:
@@ -238,7 +257,7 @@ main(int argc, char *argv[], char *envp[])
 	case MODE_LIST|MODE_INVALIDATE:
 	    ok = policy_list(&policy_plugin, nargc, nargv,
 		ISSET(sudo_mode, MODE_LONG_LIST), list_user);
-	    exit(ok != 1);
+	    gc_exit(ok != 1);
 	case MODE_EDIT:
 	case MODE_RUN:
 	    ok = policy_check(&policy_plugin, nargc, nargv, env_add,
@@ -247,7 +266,7 @@ main(int argc, char *argv[], char *envp[])
 	    if (ok != 1) {
 		if (ok == -2)
 		    usage(1);
-		exit(1); /* plugin printed error message */
+		gc_exit(1); /* plugin printed error message */
 	    }
 	    /* Reset nargv/nargc based on argv_out. */
 	    /* XXX - leaks old nargv in shell mode */
@@ -298,6 +317,7 @@ main(int argc, char *argv[], char *envp[])
 	default:
 	    sudo_fatalx(U_("unexpected sudo mode 0x%x"), sudo_mode);
     }
+
     if (WIFSIGNALED(status)) {
 	sigaction_t sa;
 
@@ -311,7 +331,7 @@ main(int argc, char *argv[], char *envp[])
     }
     sudo_debug_exit_int(__func__, __FILE__, __LINE__, sudo_debug_subsys,
 	WEXITSTATUS(status));
-    exit(WEXITSTATUS(status));
+    gc_exit(WEXITSTATUS(status));
 }
 
 int
@@ -468,8 +488,9 @@ static char **
 get_user_info(struct user_details *ud)
 {
     char *cp, **user_info, path[PATH_MAX];
+    unsigned int i = 0;
     struct passwd *pw;
-    int fd, i = 0;
+    int fd;
     debug_decl(get_user_info, SUDO_DEBUG_UTIL)
 
     memset(ud, 0, sizeof(*ud));
@@ -566,6 +587,10 @@ get_user_info(struct user_details *ud)
 
     user_info[++i] = NULL;
 
+    /* Add to list of vectors to be garbage collected at exit. */
+    if (!gc_add(GC_VECTOR, user_info))
+	goto bad;
+
     debug_return_ptr(user_info);
 bad:
     while (i--)
@@ -1145,7 +1170,7 @@ format_plugin_settings(struct plugin_container *plugin,
     struct sudo_debug_file *debug_file;
     struct sudo_settings *setting;
     char **plugin_settings;
-    unsigned int i;
+    unsigned int i = 0;
     debug_decl(format_plugin_settings, SUDO_DEBUG_PCOMM)
 
     /* Determine sudo_settings array size (including plugin_path and NULL) */
@@ -1184,6 +1209,10 @@ format_plugin_settings(struct plugin_container *plugin,
     }
     plugin_settings[++i] = NULL;
 
+    /* Add to list of vectors to be garbage collected at exit. */
+    if (!gc_add(GC_VECTOR, plugin_settings))
+	sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+
     debug_return_ptr(plugin_settings);
 bad:
     while (i--)
@@ -1441,7 +1470,81 @@ iolog_unlink(struct plugin_container *plugin)
     }
     /* Remove from io_plugins list and free. */
     TAILQ_REMOVE(&io_plugins, plugin, entries);
+    free(plugin->path);
     free(plugin);
 
     debug_return;
 }
+
+static bool
+gc_add(enum sudo_gc_types type, void *v)
+{
+#ifdef NO_LEAKS
+    struct sudo_gc_entry *gc;
+    debug_decl(gc_add, SUDO_DEBUG_MAIN)
+
+    gc = calloc(1, sizeof(*gc));
+    if (gc == NULL) {
+	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+	debug_return_bool(false);
+    }
+    switch (type) {
+    case GC_PTR:
+	gc->u.ptr = v;
+	break;
+    case GC_VECTOR:
+	gc->u.vec = v;
+	break;
+    default:
+	free(gc);
+	sudo_warnx("unexpected garbage type %d", type);
+	debug_return_bool(false);
+    }
+    gc->type = type;
+    SLIST_INSERT_HEAD(&sudo_gc_list, gc, entries);
+    debug_return_bool(true);
+#else
+    return true;
+#endif /* NO_LEAKS */
+}
+
+static void
+gc_exit(int exit_code)
+{
+#ifdef NO_LEAKS
+    struct plugin_container *plugin;
+    struct sudo_gc_entry *gc;
+    void *next;
+    char **cur;
+    debug_decl(gc_cleanup, SUDO_DEBUG_MAIN)
+
+    /* Collect garbage. */
+    SLIST_FOREACH_SAFE(gc, &sudo_gc_list, entries, next) {
+	switch (gc->type) {
+	case GC_PTR:
+	    free(gc->u.ptr);
+	    free(gc);
+	    break;
+	case GC_VECTOR:
+	    for (cur = gc->u.vec; *cur != NULL; cur++)
+		free(*cur);
+	    free(gc->u.vec);
+	    free(gc);
+	    break;
+	default:
+	    sudo_warnx("unexpected garbage type %d", gc->type);
+	}
+    }
+
+    /* Free plugin structs. */
+    free(policy_plugin.path);
+    TAILQ_FOREACH_SAFE(plugin, &io_plugins, entries, next) {
+	free(plugin->path);
+	free(plugin);
+    }
+
+    sudo_debug_exit_int(__func__, __FILE__, __LINE__, sudo_debug_subsys,
+	exit_code);
+#endif /* NO_LEAKS */
+    exit(exit_code);
+}
diff --git a/src/sudo_plugin_int.h b/src/sudo_plugin_int.h
index 3bc627330..7f23712fc 100644
--- a/src/sudo_plugin_int.h
+++ b/src/sudo_plugin_int.h
@@ -87,7 +87,7 @@ struct plugin_container {
     TAILQ_ENTRY(plugin_container) entries;
     struct sudo_conf_debug_file_list *debug_files;
     const char *name;
-    const char *path;
+    char *path;
     char * const *options;
     void *handle;
     int debug_instance;
diff --git a/src/ttyname.c b/src/ttyname.c
index 93fef39a5..c2ec5248d 100644
--- a/src/ttyname.c
+++ b/src/ttyname.c
@@ -514,11 +514,11 @@ get_process_ttyname(char *name, size_t namelen)
 		}
 	    }
 	}
-	free(line);
     }
     errno = ENOENT;
 
 done:
+    free(line);
     if (rval == NULL)
 	sudo_debug_printf(SUDO_DEBUG_WARN|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
 	    "unable to resolve tty via %s", path);