]> granicus.if.org Git - sudo/commitdiff
Add support for garbage collecting info passed to the plugin before
authorTodd C. Miller <Todd.Miller@courtesan.com>
Wed, 27 Jan 2016 22:37:15 +0000 (15:37 -0700)
committerTodd C. Miller <Todd.Miller@courtesan.com>
Wed, 27 Jan 2016 22:37:15 +0000 (15:37 -0700)
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.

src/sudo.c
src/sudo_plugin_int.h
src/ttyname.c

index 8f01cd2f5c81daeb5dc39701403bd0a81c1b7e71..dbb79dafc66d1a47b3d1cf1a96e7df2aab87dffa 100644 (file)
@@ -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);
+}
index 3bc6273300fadd5423a6f9761577cf512788a066..7f23712fc0831d69bb279666da5a45683cdc3123 100644 (file)
@@ -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;
index 93fef39a5307e3d9b254baec7b57a5b234596d95..c2ec5248dd5c383a2e1a56f683acc5907bb63476 100644 (file)
@@ -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);