Wow, that was really broken. Use mkdtemp when available to create a

2008-07-30  William Jon McCann  <jmccann@redhat.com>

	* common/Makefile.am:
	* common/gdm-common.c (gdm_make_temp_dir):
	* common/gdm-common.h:
	* common/mkdtemp.c:
	* common/mkdtemp.h:
	* configure.ac:
	* daemon/gdm-display-access-file.c (_create_xauth_file_for_user),
	(gdm_display_access_file_close):
	Wow, that was really broken.  Use mkdtemp when available
	to create a temporary directory.


svn path=/trunk/; revision=6338

1 files changed, 48 insertions, 100 deletions

diff --git a/daemon/gdm-display-access-file.c b/daemon/gdm-display-access-file.c
index 0d800b09..1f1658c9 100644
--- a/daemon/gdm-display-access-file.c
+++ b/daemon/gdm-display-access-file.c
@@ -220,77 +220,15 @@ _get_uid_and_gid_for_user (const char *username,
         return TRUE;
 }
 
-/*
- * create_temp_dir based on the mkstemp implementation from the GNU C library.
- * Copyright (C) 1991,92,93,94,95,96,97,98,99 Free Software Foundation, Inc.
- */
-static int
-create_temp_dir (char *tmpl,
-                 int    permissions)
-{
-        char             *XXXXXX;
-        int               count;
-        int               fd;
-        static const char letters[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
-        static const int  NLETTERS = sizeof (letters) - 1;
-        glong             value;
-        GTimeVal          tv;
-        static int        counter = 0;
-
-        /* find the last occurrence of "XXXXXX" */
-        XXXXXX = g_strrstr (tmpl, "XXXXXX");
-
-        if (!XXXXXX || strncmp (XXXXXX, "XXXXXX", 6)) {
-                errno = EINVAL;
-                return -1;
-        }
-
-        /* Get some more or less random data.  */
-        g_get_current_time (&tv);
-        value = (tv.tv_usec ^ tv.tv_sec) + counter++;
-
-        for (count = 0; count < 100; value += 7777, ++count) {
-                glong v = value;
-
-                /* Fill in the random bits.  */
-                XXXXXX[0] = letters[v % NLETTERS];
-                v /= NLETTERS;
-                XXXXXX[1] = letters[v % NLETTERS];
-                v /= NLETTERS;
-                XXXXXX[2] = letters[v % NLETTERS];
-                v /= NLETTERS;
-                XXXXXX[3] = letters[v % NLETTERS];
-                v /= NLETTERS;
-                XXXXXX[4] = letters[v % NLETTERS];
-                v /= NLETTERS;
-                XXXXXX[5] = letters[v % NLETTERS];
-
-                /* tmpl is in UTF-8 on Windows, thus use g_mkdir() */
-                fd = g_mkdir (tmpl, permissions);
-                if (fd >= 0) {
-                        return fd;
-                } else if (errno != EEXIST) {
-                        /* Any other error will apply also to other names we might
-                         *  try, and there are 2^32 or so of them, so give up now.
-                         */
-                        return -1;
-                }
-        }
-
-        /* We got out of the loop because we ran out of combinations to try.  */
-        errno = EEXIST;
-        return -1;
-}
-
 static FILE *
 _create_xauth_file_for_user (const char  *username,
                              char       **filename,
                              GError     **error)
 {
         char   *template;
+        char   *dir_name;
         char   *auth_filename;
-        int     dir_fd;
-        int     file_fd;
+        int     fd;
         FILE   *fp;
         uid_t   uid;
         gid_t   gid;
@@ -302,8 +240,7 @@ _create_xauth_file_for_user (const char  *username,
         template = NULL;
         auth_filename = NULL;
         fp = NULL;
-        dir_fd = -1;
-        file_fd = -1;
+        fd = -1;
 
         /* Create directory if not exist, then set permission 01775 and ownership root:gdm */
         if (g_file_test (GDM_XAUTH_DIR, G_FILE_TEST_IS_DIR) == FALSE) {
@@ -318,7 +255,10 @@ _create_xauth_file_for_user (const char  *username,
 
                 g_chmod (GDM_XAUTH_DIR, S_ISVTX | S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH);
                 _get_uid_and_gid_for_user ("gdm", &uid, &gid);
-                chown (GDM_XAUTH_DIR, 0, gid);
+                if (chown (GDM_XAUTH_DIR, 0, gid) != 0) {
+                        g_warning ("Unable to change owner of '%s'",
+                                   GDM_XAUTH_DIR);
+                }
         } else {
                 /* if it does exist make sure it has correct mode 01775 */
                 g_chmod (GDM_XAUTH_DIR, S_ISVTX | S_IRWXU |S_IRWXG | S_IROTH | S_IXOTH);
@@ -338,68 +278,79 @@ _create_xauth_file_for_user (const char  *username,
                                     "/auth-for-%s-XXXXXX",
                                     username);
 
+        g_debug ("GdmDisplayAccessFile: creating xauth directory %s", template);
         /* Initially create with mode 01700 then later chmod after we create database */
-        dir_fd = create_temp_dir (template, S_ISVTX | S_IRWXU);
-        if (dir_fd < 0) {
+        errno = 0;
+        dir_name = gdm_make_temp_dir (template);
+        if (dir_name == NULL) {
                 g_set_error (error,
                              G_FILE_ERROR,
                              g_file_error_from_errno (errno),
-                             "%s", g_strerror (errno));
+                             "Unable to create temp dir from tempalte '%s': %s",
+                             template,
+                             g_strerror (errno));
                 goto out;
         }
 
-        g_debug ("GdmDisplayAccessFile: chowning %s to %u:%u", template, (guint)uid, (guint)gid);
-        if (fchown (dir_fd, uid, gid) < 0) {
+        g_debug ("GdmDisplayAccessFile: chowning %s to %u:%u",
+                 dir_name, (guint)uid, (guint)gid);
+        errno = 0;
+        if (chown (dir_name, uid, gid) < 0) {
                 g_set_error (error,
                              G_FILE_ERROR,
                              g_file_error_from_errno (errno),
-                             "%s", g_strerror (errno));
-                close (dir_fd);
-                dir_fd = -1;
+                             "Unable to change permission of '%s': %s",
+                             dir_name,
+                             g_strerror (errno));
                 goto out;
         }
-        close (dir_fd);
-        dir_fd = -1;
 
-        auth_filename = g_build_filename (template, "database", NULL);
+        auth_filename = g_build_filename (dir_name, "database", NULL);
 
         g_debug ("GdmDisplayAccessFile: creating %s", auth_filename);
         /* mode 00600 */
-        file_fd = g_open (auth_filename,
-                          O_RDWR | O_CREAT | O_EXCL | O_BINARY,
-                          S_IRUSR | S_IWUSR);
+        errno = 0;
+        fd = g_open (auth_filename,
+                     O_RDWR | O_CREAT | O_EXCL | O_BINARY,
+                     S_IRUSR | S_IWUSR);
 
-        if (file_fd < 0) {
+        if (fd < 0) {
                 g_set_error (error,
                              G_FILE_ERROR,
                              g_file_error_from_errno (errno),
-                             "%s", g_strerror (errno));
+                             "Unable to open '%s': %s",
+                             auth_filename,
+                             g_strerror (errno));
                 goto out;
         }
 
         g_debug ("GdmDisplayAccessFile: chowning %s to %u:%u", auth_filename, (guint)uid, (guint)gid);
-        if (fchown (file_fd, uid, gid) < 0) {
+        errno = 0;
+        if (fchown (fd, uid, gid) < 0) {
                 g_set_error (error,
                              G_FILE_ERROR,
                              g_file_error_from_errno (errno),
-                             "%s", g_strerror (errno));
-                close (file_fd);
-                file_fd = -1;
+                             "Unable to change owner for '%s': %s",
+                             auth_filename,
+                             g_strerror (errno));
+                close (fd);
+                fd = -1;
                 goto out;
         }
 
         /* now open up permissions on per-session directory */
-        g_debug ("GdmDisplayAccessFile: chmoding %s to 1777", template);
-        g_chmod (template, S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO);
+        g_debug ("GdmDisplayAccessFile: chmoding %s to 1777", dir_name);
+        g_chmod (dir_name, S_ISVTX | S_IRWXU | S_IRWXG | S_IRWXO);
 
-        fp = fdopen (file_fd, "w");
+        errno = 0;
+        fp = fdopen (fd, "w");
         if (fp == NULL) {
                 g_set_error (error,
                              G_FILE_ERROR,
                              g_file_error_from_errno (errno),
                              "%s", g_strerror (errno));
-                close (file_fd);
-                file_fd = -1;
+                close (fd);
+                fd = -1;
                 goto out;
         }
 
@@ -407,15 +358,12 @@ _create_xauth_file_for_user (const char  *username,
         auth_filename = NULL;
 
         /* don't close it */
-        file_fd = -1;
+        fd = -1;
 out:
         g_free (template);
         g_free (auth_filename);
-        if (dir_fd != -1) {
-                close (dir_fd);
-        }
-        if (file_fd != -1) {
-                close (file_fd);
+        if (fd != -1) {
+                close (fd);
         }
 
         return fp;
@@ -636,7 +584,7 @@ gdm_display_access_file_close (GdmDisplayAccessFile  *file)
         if (g_unlink (file->priv->path) != 0) {
                 g_warning ("GdmDisplayAccessFile: Unable to remove X11 authority database '%s': %s",
                            file->priv->path,
-                           g_file_error_from_errno (errno));
+                           g_strerror (errno));
         }
 
         /* still try to remove dir even if file remove failed,
@@ -648,7 +596,7 @@ gdm_display_access_file_close (GdmDisplayAccessFile  *file)
                 if (g_rmdir (auth_dir) != 0) {
                         g_warning ("GdmDisplayAccessFile: Unable to remove X11 authority directory '%s': %s",
                                    auth_dir,
-                                   g_file_error_from_errno (errno));
+                                   g_strerror (errno));
                 }
                 g_free (auth_dir);
         }