diff options
author | Robert Ancell <robert.ancell@canonical.com> | 2014-02-26 11:12:22 +1300 |
---|---|---|
committer | Robert Ancell <robert.ancell@canonical.com> | 2014-02-26 11:12:22 +1300 |
commit | 2b899d2c371eaeed3181a94a37266e8374e8a45f (patch) | |
tree | 4aa007faa44944a91ffdbb5598dcd51340d51525 /src | |
parent | eb669026e7fd106bd9bb176110b308562801b409 (diff) | |
download | lightdm-2b899d2c371eaeed3181a94a37266e8374e8a45f.tar.gz |
Be extra careful not to call any non thread safe function after a fork
Diffstat (limited to 'src')
-rw-r--r-- | src/process.c | 130 | ||||
-rw-r--r-- | src/process.h | 7 | ||||
-rw-r--r-- | src/seat.c | 2 | ||||
-rw-r--r-- | src/session-child.c | 64 | ||||
-rw-r--r-- | src/session.c | 21 | ||||
-rw-r--r-- | src/unity-system-compositor.c | 40 | ||||
-rw-r--r-- | src/x-server-local.c | 40 | ||||
-rw-r--r-- | src/x-server-xvnc.c | 34 |
8 files changed, 159 insertions, 179 deletions
diff --git a/src/process.c b/src/process.c index 542492bc..3010eb0e 100644 --- a/src/process.c +++ b/src/process.c @@ -23,7 +23,6 @@ #include "process.h" enum { - RUN, GOT_DATA, GOT_SIGNAL, STOPPED, @@ -32,10 +31,18 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; struct ProcessPrivate -{ +{ + /* Function to run inside subprocess before exec */ + ProcessRunFunc run_func; + gpointer run_func_data; + + /* File to log to */ + gchar *log_file; + gboolean log_stdout; + /* Command to run */ gchar *command; - + /* TRUE to clear the environment in this process */ gboolean clear_environment; @@ -67,16 +74,28 @@ process_get_current (void) if (current_process) return current_process; - current_process = process_new (); + current_process = process_new (NULL, NULL); current_process->priv->pid = getpid (); return current_process; } Process * -process_new (void) +process_new (ProcessRunFunc run_func, gpointer run_func_data) { - return g_object_new (PROCESS_TYPE, NULL); + Process *process = g_object_new (PROCESS_TYPE, NULL); + process->priv->run_func = run_func; + process->priv->run_func_data = run_func_data; + return process; +} + +void +process_set_log_file (Process *process, const gchar *path, gboolean log_stdout) +{ + g_return_if_fail (process != NULL); + g_free (process->priv->log_file); + process->priv->log_file = g_strdup (path); + process->priv->log_stdout = log_stdout; } void @@ -150,57 +169,88 @@ process_watch_cb (GPid pid, gint status, gpointer data) g_signal_emit (process, signals[STOPPED], 0); } -static void -process_run (Process *process) +gboolean +process_start (Process *process, gboolean block) { gint argc; gchar **argv; - GHashTableIter iter; - gpointer key, value; + gchar **env_keys, **env_values; + guint i, env_length; + pid_t pid; + int log_fd = -1; GError *error = NULL; + g_return_val_if_fail (process != NULL, FALSE); + g_return_val_if_fail (process->priv->command != NULL, FALSE); + g_return_val_if_fail (process->priv->pid == 0, FALSE); + if (!g_shell_parse_argv (process->priv->command, &argc, &argv, &error)) { g_warning ("Error parsing command %s: %s", process->priv->command, error->message); - _exit (EXIT_FAILURE); + return FALSE; + } + + if (process->priv->log_file) + { + gchar *old_filename; + + /* Move old file out of the way */ + old_filename = g_strdup_printf ("%s.old", process->priv->log_file); + rename (process->priv->log_file, old_filename); + g_free (old_filename); + + /* Create new file and log to it */ + log_fd = g_open (process->priv->log_file, O_WRONLY | O_CREAT | O_TRUNC, 0600); + if (log_fd < 0) + g_warning ("Failed to open log file %s: %s", process->priv->log_file, g_strerror (errno)); } - if (process->priv->clear_environment) + /* Work out variables to set */ + env_keys = (gchar **) g_hash_table_get_keys_as_array (process->priv->env, &env_length); + env_values = g_malloc (sizeof (gchar *) * env_length); + for (i = 0; i < env_length; i++) + env_values[i] = g_hash_table_lookup (process->priv->env, env_keys[i]); + + pid = fork (); + if (pid == 0) + { + /* Do custom setup */ + if (process->priv->run_func) + process->priv->run_func (process, process->priv->run_func_data); + + /* Redirect output to logfile */ + if (log_fd >= 0) + { + if (process->priv->log_stdout) + dup2 (log_fd, STDOUT_FILENO); + dup2 (log_fd, STDERR_FILENO); + close (log_fd); + } + + /* Set environment */ + if (process->priv->clear_environment) #ifdef HAVE_CLEARENV - clearenv (); + clearenv (); #else - environ = NULL; + environ = NULL; #endif - - g_hash_table_iter_init (&iter, process->priv->env); - while (g_hash_table_iter_next (&iter, &key, &value)) - g_setenv ((gchar *)key, (gchar *)value, TRUE); + for (i = 0; i < env_length; i++) + setenv (env_keys[i], env_values[i], TRUE); - execvp (argv[0], argv); - - g_warning ("Error executing child process %s: %s", argv[0], g_strerror (errno)); - _exit (EXIT_FAILURE); -} - -gboolean -process_start (Process *process, gboolean block) -{ - pid_t pid; + execvp (argv[0], argv); + _exit (EXIT_FAILURE); + } - g_return_val_if_fail (process != NULL, FALSE); - g_return_val_if_fail (process->priv->command != NULL, FALSE); - g_return_val_if_fail (process->priv->pid == 0, FALSE); + close (log_fd); + g_free (env_keys); + g_free (env_values); - pid = fork (); if (pid < 0) { g_warning ("Failed to fork: %s", strerror (errno)); return FALSE; } - if (pid == 0) - g_signal_emit (process, signals[RUN], 0); - g_debug ("Launching process %d: %s", pid, process->priv->command); process->priv->pid = pid; @@ -295,6 +345,7 @@ process_finalize (GObject *object) if (self->priv->pid > 0) g_hash_table_remove (processes, GINT_TO_POINTER (self->priv->pid)); + g_free (self->priv->log_file); g_free (self->priv->command); g_hash_table_unref (self->priv->env); if (self->priv->quit_timeout) @@ -349,20 +400,11 @@ process_class_init (ProcessClass *klass) GObjectClass *object_class = G_OBJECT_CLASS (klass); struct sigaction action; - klass->run = process_run; klass->stopped = process_stopped; object_class->finalize = process_finalize; g_type_class_add_private (klass, sizeof (ProcessPrivate)); - signals[RUN] = - g_signal_new ("run", - G_TYPE_FROM_CLASS (klass), - G_SIGNAL_RUN_LAST, - G_STRUCT_OFFSET (ProcessClass, run), - NULL, NULL, - NULL, - G_TYPE_NONE, 0); signals[GOT_DATA] = g_signal_new ("got-data", G_TYPE_FROM_CLASS (klass), diff --git a/src/process.h b/src/process.h index 54c3776b..1fcde7a8 100644 --- a/src/process.h +++ b/src/process.h @@ -32,17 +32,20 @@ typedef struct typedef struct { GObjectClass parent_class; - void (*run)(Process *process); void (*got_data)(Process *process); void (*got_signal)(Process *process, int signum); void (*stopped)(Process *process); } ProcessClass; +typedef void (*ProcessRunFunc)(Process *process, gpointer user_data); + GType process_get_type (void); Process *process_get_current (void); -Process *process_new (void); +Process *process_new (ProcessRunFunc run_func, gpointer run_func_data); + +void process_set_log_file (Process *process, const gchar *path, gboolean log_stdout); void process_set_clear_environment (Process *process, gboolean clear_environment); @@ -269,7 +269,7 @@ run_script (Seat *seat, DisplayServer *display_server, const gchar *script_name, Process *script; gboolean result = FALSE; - script = process_new (); + script = process_new (NULL, NULL); process_set_command (script, script_name); diff --git a/src/session-child.c b/src/session-child.c index 6d6d05e9..c1c84e5d 100644 --- a/src/session-child.c +++ b/src/session-child.c @@ -234,6 +234,9 @@ session_child_run (int argc, char **argv) "LANG", NULL }; + gid_t gid; + uid_t uid; + const gchar *home_directory; GError *error = NULL; #if !defined(GLIB_VERSION_2_36) @@ -417,17 +420,21 @@ session_child_run (int argc, char **argv) /* Redirect stderr to a log file */ if (log_filename) - log_backup_filename = g_strdup_printf ("%s.old", log_filename); - if (!log_filename) { - fd = open ("/dev/null", O_WRONLY); - dup2 (fd, STDERR_FILENO); - close (fd); + log_backup_filename = g_strdup_printf ("%s.old", log_filename); + if (g_path_is_absolute (log_filename)) + { + rename (log_filename, log_backup_filename); + fd = open (log_filename, O_WRONLY | O_APPEND | O_CREAT, 0600); + dup2 (fd, STDERR_FILENO); + close (fd); + g_free (log_filename); + log_filename = NULL; + } } - else if (g_path_is_absolute (log_filename)) + else { - rename (log_filename, log_backup_filename); - fd = open (log_filename, O_WRONLY | O_APPEND | O_CREAT, 0600); + fd = open ("/dev/null", O_WRONLY); dup2 (fd, STDERR_FILENO); close (fd); } @@ -532,53 +539,46 @@ session_child_run (int argc, char **argv) signal (SIGTERM, signal_cb); /* Run the command as the authenticated user */ - child_pid = fork (); + uid = user_get_uid (user); + gid = user_get_gid (user); + home_directory = user_get_home_directory (user); + child_pid = fork (); if (child_pid == 0) { - // FIXME: This is not thread safe (particularly the printfs) - /* Make this process its own session */ if (setsid () < 0) - g_printerr ("Failed to make process a new session: %s\n", strerror (errno)); + _exit (errno); /* Change to this user */ if (getuid () == 0) { - if (setgid (user_get_gid (user)) != 0) - { - g_printerr ("Failed to set group ID to %d: %s\n", user_get_gid (user), strerror (errno)); - _exit (EXIT_FAILURE); - } + if (setgid (gid) != 0) + _exit (errno); - if (setuid (user_get_uid (user)) != 0) - { - g_printerr ("Failed to set user ID to %d: %s\n", user_get_uid (user), strerror (errno)); - _exit (EXIT_FAILURE); - } + if (setuid (uid) != 0) + _exit (errno); } /* Change working directory */ /* NOTE: This must be done after the permissions are changed because NFS filesystems can * be setup so the local root user accesses the NFS files as 'nobody'. If the home directories * are not system readable then the chdir can fail */ - if (chdir (user_get_home_directory (user)) != 0) - { - g_printerr ("Failed to change to home directory %s: %s\n", user_get_home_directory (user), strerror (errno)); - _exit (EXIT_FAILURE); - } + if (chdir (home_directory) != 0) + _exit (errno); - /* Redirect stderr to a log file */ - if (log_filename && !g_path_is_absolute (log_filename)) + if (log_filename) { rename (log_filename, log_backup_filename); fd = open (log_filename, O_WRONLY | O_APPEND | O_CREAT, 0600); - dup2 (fd, STDERR_FILENO); - close (fd); + if (fd >= 0) + { + dup2 (fd, STDERR_FILENO); + close (fd); + } } /* Run the command */ execve (command_argv[0], command_argv, pam_getenvlist (pam_handle)); - g_printerr ("Failed to run command: %s\n", strerror (errno)); _exit (EXIT_FAILURE); } diff --git a/src/session.c b/src/session.c index bc3928be..88616f53 100644 --- a/src/session.c +++ b/src/session.c @@ -549,6 +549,7 @@ session_real_start (Session *session) int version; int to_child_pipe[2], from_child_pipe[2]; int to_child_output, from_child_input; + gchar *arg0, *arg1; g_return_val_if_fail (session->priv->pid == 0, FALSE); @@ -581,24 +582,26 @@ session_real_start (Session *session) } /* Run the child */ + arg0 = g_strdup_printf ("%d", to_child_output); + arg1 = g_strdup_printf ("%d", from_child_input); session->priv->pid = fork (); - if (session->priv->pid < 0) - { - g_debug ("Failed to fork session child process: %s", strerror (errno)); - return FALSE; - } - if (session->priv->pid == 0) { /* Run us again in session child mode */ execlp ("lightdm", "lightdm", "--session-child", - g_strdup_printf ("%d", to_child_output), - g_strdup_printf ("%d", from_child_input), - NULL); + arg0, arg1, NULL); _exit (EXIT_FAILURE); } + g_free (arg0); + g_free (arg1); + + if (session->priv->pid < 0) + { + g_debug ("Failed to fork session child process: %s", strerror (errno)); + return FALSE; + } /* Hold a reference on this object until the child process terminates so we * can handle the watch callback even if it is no longer used. Otherwise a diff --git a/src/unity-system-compositor.c b/src/unity-system-compositor.c index 13362095..9624f450 100644 --- a/src/unity-system-compositor.c +++ b/src/unity-system-compositor.c @@ -28,9 +28,6 @@ struct UnitySystemCompositorPrivate { /* Compositor process */ Process *process; - - /* File to log to */ - gchar *log_file; /* Command to run the compositor */ gchar *command; @@ -313,7 +310,7 @@ read_cb (GIOChannel *source, GIOCondition condition, gpointer data) } static void -run_cb (Process *process, UnitySystemCompositor *compositor) +run_cb (Process *process, gpointer user_data) { int fd; @@ -321,29 +318,6 @@ run_cb (Process *process, UnitySystemCompositor *compositor) fd = open ("/dev/null", O_RDONLY); dup2 (fd, STDIN_FILENO); close (fd); - - /* Redirect output to logfile */ - if (compositor->priv->log_file) - { - int fd; - gchar *old_filename; - - /* Move old file out of the way */ - old_filename = g_strdup_printf ("%s.old", compositor->priv->log_file); - rename (compositor->priv->log_file, old_filename); - g_free (old_filename); - - /* Create new file and log to it */ - fd = g_open (compositor->priv->log_file, O_WRONLY | O_CREAT | O_TRUNC, 0600); - if (fd < 0) - l_warning (compositor, "Failed to open log file %s: %s", compositor->priv->log_file, g_strerror (errno)); - else - { - dup2 (fd, STDOUT_FILENO); - dup2 (fd, STDERR_FILENO); - close (fd); - } - } } static gboolean @@ -383,7 +357,7 @@ unity_system_compositor_start (DisplayServer *server) { UnitySystemCompositor *compositor = UNITY_SYSTEM_COMPOSITOR (server); gboolean result; - gchar *dir, *command, *absolute_command, *value; + gchar *dir, *log_file, *command, *absolute_command, *value; g_return_val_if_fail (compositor->priv->process == NULL, FALSE); @@ -408,12 +382,14 @@ unity_system_compositor_start (DisplayServer *server) /* Setup logging */ dir = config_get_string (config_get_instance (), "LightDM", "log-directory"); - compositor->priv->log_file = g_build_filename (dir, "unity-system-compositor.log", NULL); - l_debug (compositor, "Logging to %s", compositor->priv->log_file); + log_file = g_build_filename (dir, "unity-system-compositor.log", NULL); + l_debug (compositor, "Logging to %s", log_file); g_free (dir); /* Setup environment */ - compositor->priv->process = process_new (); + compositor->priv->process = process_new (run_cb, compositor); + process_set_log_file (compositor->priv->process, log_file, TRUE); + g_free (log_file); process_set_clear_environment (compositor->priv->process, TRUE); process_set_env (compositor->priv->process, "XDG_SEAT", "seat0"); value = g_strdup_printf ("%d", compositor->priv->vt); @@ -435,7 +411,6 @@ unity_system_compositor_start (DisplayServer *server) process_set_command (compositor->priv->process, absolute_command); g_free (absolute_command); g_signal_connect (compositor->priv->process, "stopped", G_CALLBACK (stopped_cb), compositor); - g_signal_connect (compositor->priv->process, "run", G_CALLBACK (run_cb), compositor); result = process_start (compositor->priv->process, FALSE); /* Close compostor ends of the pipes */ @@ -485,7 +460,6 @@ unity_system_compositor_finalize (GObject *object) g_signal_handlers_disconnect_matched (self->priv->process, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, self); g_object_unref (self->priv->process); } - g_free (self->priv->log_file); g_free (self->priv->command); g_free (self->priv->socket); if (self->priv->have_vt_ref) diff --git a/src/x-server-local.c b/src/x-server-local.c index efe1227e..11d6fd1f 100644 --- a/src/x-server-local.c +++ b/src/x-server-local.c @@ -27,9 +27,6 @@ struct XServerLocalPrivate { /* X server process */ Process *x_server_process; - - /* File to log to */ - gchar *log_file; /* Command to run the X server */ gchar *command; @@ -326,7 +323,7 @@ get_absolute_command (const gchar *command) } static void -run_cb (Process *process, XServerLocal *server) +run_cb (Process *process, gpointer user_data) { int fd; @@ -335,29 +332,6 @@ run_cb (Process *process, XServerLocal *server) dup2 (fd, STDIN_FILENO); close (fd); - /* Redirect output to logfile */ - if (server->priv->log_file) - { - int fd; - gchar *old_filename; - - /* Move old file out of the way */ - old_filename = g_strdup_printf ("%s.old", server->priv->log_file); - rename (server->priv->log_file, old_filename); - g_free (old_filename); - - /* Create new file and log to it */ - fd = g_open (server->priv->log_file, O_WRONLY | O_CREAT | O_TRUNC, 0600); - if (fd < 0) - l_warning (server, "Failed to open log file %s: %s", server->priv->log_file, g_strerror (errno)); - else - { - dup2 (fd, STDOUT_FILENO); - dup2 (fd, STDERR_FILENO); - close (fd); - } - } - /* Set SIGUSR1 to ignore so the X server can indicate it when it is ready */ signal (SIGUSR1, SIG_IGN); } @@ -439,7 +413,7 @@ x_server_local_start (DisplayServer *display_server) { XServerLocal *server = X_SERVER_LOCAL (display_server); gboolean result; - gchar *filename, *dir, *absolute_command; + gchar *filename, *dir, *log_file, *absolute_command; GString *command; g_return_val_if_fail (server->priv->x_server_process == NULL, FALSE); @@ -448,17 +422,18 @@ x_server_local_start (DisplayServer *display_server) g_return_val_if_fail (server->priv->command != NULL, FALSE); - server->priv->x_server_process = process_new (); + server->priv->x_server_process = process_new (run_cb, server); process_set_clear_environment (server->priv->x_server_process, TRUE); - g_signal_connect (server->priv->x_server_process, "run", G_CALLBACK (run_cb), server); g_signal_connect (server->priv->x_server_process, "got-signal", G_CALLBACK (got_signal_cb), server); g_signal_connect (server->priv->x_server_process, "stopped", G_CALLBACK (stopped_cb), server); /* Setup logging */ filename = g_strdup_printf ("%s.log", display_server_get_name (display_server)); dir = config_get_string (config_get_instance (), "LightDM", "log-directory"); - server->priv->log_file = g_build_filename (dir, filename, NULL); - l_debug (display_server, "Logging to %s", server->priv->log_file); + log_file = g_build_filename (dir, filename, NULL); + process_set_log_file (server->priv->x_server_process, log_file, TRUE); + l_debug (display_server, "Logging to %s", log_file); + g_free (log_file); g_free (filename); g_free (dir); @@ -577,7 +552,6 @@ x_server_local_finalize (GObject *object) g_signal_handlers_disconnect_matched (self->priv->x_server_process, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, self); g_object_unref (self->priv->x_server_process); } - g_free (self->priv->log_file); g_free (self->priv->command); g_free (self->priv->config_file); g_free (self->priv->layout); diff --git a/src/x-server-xvnc.c b/src/x-server-xvnc.c index 0897725d..884489dd 100644 --- a/src/x-server-xvnc.c +++ b/src/x-server-xvnc.c @@ -26,9 +26,6 @@ struct XServerXVNCPrivate { /* X server process */ Process *x_server_process; - - /* File to log to */ - gchar *log_file; /* Command to run the X server */ gchar *command; @@ -130,28 +127,15 @@ get_absolute_command (const gchar *command) } static void -run_cb (Process *process, XServerXVNC *server) +run_cb (Process *process, gpointer user_data) { + XServerXVNC *server = user_data; + /* Connect input */ dup2 (server->priv->socket_fd, STDIN_FILENO); dup2 (server->priv->socket_fd, STDOUT_FILENO); close (server->priv->socket_fd); - /* Redirect output to logfile */ - if (server->priv->log_file) - { - int fd; - - fd = g_open (server->priv->log_file, O_WRONLY | O_CREAT | O_TRUNC, 0600); - if (fd < 0) - l_warning (server, "Failed to open log file %s: %s", server->priv->log_file, g_strerror (errno)); - else - { - dup2 (fd, STDERR_FILENO); - close (fd); - } - } - /* Set SIGUSR1 to ignore so the X server can indicate it when it is ready */ signal (SIGUSR1, SIG_IGN); } @@ -200,7 +184,7 @@ x_server_xvnc_start (DisplayServer *display_server) XServerXVNC *server = X_SERVER_XVNC (display_server); XAuthority *authority; gboolean result; - gchar *filename, *run_dir, *dir, *absolute_command; + gchar *filename, *run_dir, *dir, *log_file, *absolute_command; GString *command; gchar hostname[1024], *number; GError *error = NULL; @@ -209,17 +193,18 @@ x_server_xvnc_start (DisplayServer *display_server) server->priv->got_signal = FALSE; - server->priv->x_server_process = process_new (); + server->priv->x_server_process = process_new (run_cb, server); process_set_clear_environment (server->priv->x_server_process, TRUE); - g_signal_connect (server->priv->x_server_process, "run", G_CALLBACK (run_cb), server); g_signal_connect (server->priv->x_server_process, "got-signal", G_CALLBACK (got_signal_cb), server); g_signal_connect (server->priv->x_server_process, "stopped", G_CALLBACK (stopped_cb), server); /* Setup logging */ filename = g_strdup_printf ("%s.log", display_server_get_name (display_server)); dir = config_get_string (config_get_instance (), "LightDM", "log-directory"); - server->priv->log_file = g_build_filename (dir, filename, NULL); - l_debug (display_server, "Logging to %s", server->priv->log_file); + log_file = g_build_filename (dir, filename, NULL); + process_set_log_file (server->priv->x_server_process, log_file, FALSE); + l_debug (display_server, "Logging to %s", log_file); + g_free (log_file); g_free (filename); g_free (dir); @@ -314,7 +299,6 @@ x_server_xvnc_finalize (GObject *object) g_object_unref (self->priv->x_server_process); g_free (self->priv->command); g_free (self->priv->authority_file); - g_free (self->priv->log_file); G_OBJECT_CLASS (x_server_xvnc_parent_class)->finalize (object); } |