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

8 files changed, 332 insertions, 100 deletions

diff --git a/ChangeLog b/ChangeLog
index 5d5d1a9d..04709506 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 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.
+
+2008-07-30  William Jon McCann  <jmccann@redhat.com>
+
 	* daemon/gdm-display-access-file.c (gdm_display_access_file_close):
 	Remove the X authorizations directory when we're done
 	with it.
diff --git a/common/Makefile.am b/common/Makefile.am
index f698bcef..101b3f4e 100644
--- a/common/Makefile.am
+++ b/common/Makefile.am
@@ -60,6 +60,12 @@ gdm-marshal.c: gdm-marshal.list
 gdm-marshal.h: gdm-marshal.list
 	@GLIB_GENMARSHAL@ $< --prefix=gdm_marshal --header > $@
 
+if MKDTEMP_MISSING
+MKDTEMP_FILES = mkdtemp.c mkdtemp.h
+else
+MKDTEMP_FILES =
+endif
+
 libgdmcommon_la_SOURCES =	\
 	gdm-address.h		\
 	gdm-address.c		\
@@ -90,6 +96,7 @@ libgdmcommon_la_SOURCES =	\
 	gdm-md5.c		\
 	gdm-signal-handler.h	\
 	gdm-signal-handler.c	\
+	$(MKDTEMP_FILES) 	\
 	$(NULL)
 
 libgdmcommon_la_CPPFLAGS = 		\
@@ -147,5 +154,7 @@ CLEANFILES =			\
 EXTRA_DIST = 			\
 	gdm-settings.xml	\
 	gdm-marshal.list	\
+	mkdtemp.c 		\
+	mkdtemp.h		\
 	$(gdm_DATA)		\
 	$(NULL)
diff --git a/common/gdm-common.c b/common/gdm-common.c
index e96bd434..278ed1e9 100644
--- a/common/gdm-common.c
+++ b/common/gdm-common.c
@@ -33,6 +33,16 @@
 
 #include "gdm-common.h"
 
+#ifndef HAVE_MKDTEMP
+#include "mkdtemp.h"
+#endif
+
+char *
+gdm_make_temp_dir (const char *template)
+{
+        return mkdtemp (template);
+}
+
 gboolean
 gdm_is_version_unstable (void)
 {
diff --git a/common/gdm-common.h b/common/gdm-common.h
index c3122815..328d511c 100644
--- a/common/gdm-common.h
+++ b/common/gdm-common.h
@@ -34,6 +34,8 @@ int            gdm_wait_on_pid           (int pid);
 int            gdm_signal_pid            (int pid,
                                           int signal);
 
+char *         gdm_make_temp_dir         (const char    *template);
+
 gboolean       gdm_string_hex_encode     (const GString *source,
                                           int            start,
                                           GString       *dest,
diff --git a/common/mkdtemp.c b/common/mkdtemp.c
new file mode 100644
index 00000000..15081020
--- /dev/null
+++ b/common/mkdtemp.c
@@ -0,0 +1,201 @@
+/* Copyright (C) 1999, 2001-2002 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Library General Public License as
+   published by the Free Software Foundation; either version 2 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Library General Public License for more details.
+
+   You should have received a copy of the GNU Library General Public
+   License along with the GNU C Library; see the file COPYING.LIB.  If not,
+   write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+/* Extracted from misc/mkdtemp.c and sysdeps/posix/tempname.c.  */
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+/* Specification.  */
+#include "mkdtemp.h"
+
+#include <errno.h>
+#ifndef __set_errno
+# define __set_errno(Val) errno = (Val)
+#endif
+
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <stdio.h>
+#ifndef TMP_MAX
+# define TMP_MAX 238328
+#endif
+
+#if HAVE_STDINT_H_WITH_UINTMAX || _LIBC
+# include <stdint.h>
+#endif
+
+#if HAVE_INTTYPES_H_WITH_UINTMAX || _LIBC
+# include <inttypes.h>
+#endif
+
+#if HAVE_UNISTD_H || _LIBC
+# include <unistd.h>
+#endif
+
+#if HAVE_GETTIMEOFDAY || _LIBC
+# if HAVE_SYS_TIME_H || _LIBC
+#  include <sys/time.h>
+# endif
+#else
+# if HAVE_TIME_H || _LIBC
+#  include <time.h>
+# endif
+#endif
+
+#include <sys/stat.h>
+#if STAT_MACROS_BROKEN
+# undef S_ISDIR
+#endif
+#if !defined S_ISDIR && defined S_IFDIR
+# define S_ISDIR(mode) (((mode) & S_IFMT) == S_IFDIR)
+#endif
+#if !S_IRUSR && S_IREAD
+# define S_IRUSR S_IREAD
+#endif
+#if !S_IRUSR
+# define S_IRUSR 00400
+#endif
+#if !S_IWUSR && S_IWRITE
+# define S_IWUSR S_IWRITE
+#endif
+#if !S_IWUSR
+# define S_IWUSR 00200
+#endif
+#if !S_IXUSR && S_IEXEC
+# define S_IXUSR S_IEXEC
+#endif
+#if !S_IXUSR
+# define S_IXUSR 00100
+#endif
+
+#if !_LIBC
+# define __getpid getpid
+# define __gettimeofday gettimeofday
+# define __mkdir mkdir
+#endif
+
+/* Use the widest available unsigned type if uint64_t is not
+   available.  The algorithm below extracts a number less than 62**6
+   (approximately 2**35.725) from uint64_t, so ancient hosts where
+   uintmax_t is only 32 bits lose about 3.725 bits of randomness,
+   which is better than not having mkstemp at all.  */
+#if !defined UINT64_MAX && !defined uint64_t
+# define uint64_t uintmax_t
+#endif
+
+/* These are the characters used in temporary filenames.  */
+static const char letters[] =
+"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
+
+/* Generate a temporary file name based on TMPL.  TMPL must match the
+   rules for mk[s]temp (i.e. end in "XXXXXX").  The name constructed
+   does not exist at the time of the call to __gen_tempname.  TMPL is
+   overwritten with the result.
+
+   KIND is:
+   __GT_DIR:		create a directory, which will be mode 0700.
+
+   We use a clever algorithm to get hard-to-predict names. */
+static int
+gen_tempname (tmpl)
+     char *tmpl;
+{
+  int len;
+  char *XXXXXX;
+  static uint64_t value;
+  uint64_t random_time_bits;
+  int count, fd = -1;
+  int save_errno = errno;
+
+  len = strlen (tmpl);
+  if (len < 6 || strcmp (&tmpl[len - 6], "XXXXXX"))
+    {
+      __set_errno (EINVAL);
+      return -1;
+    }
+
+  /* This is where the Xs start.  */
+  XXXXXX = &tmpl[len - 6];
+
+  /* Get some more or less random data.  */
+#ifdef RANDOM_BITS
+  RANDOM_BITS (random_time_bits);
+#else
+# if HAVE_GETTIMEOFDAY || _LIBC
+  {
+    struct timeval tv;
+    __gettimeofday (&tv, NULL);
+    random_time_bits = ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec;
+  }
+# else
+  random_time_bits = time (NULL);
+# endif
+#endif
+  value += random_time_bits ^ __getpid ();
+
+  for (count = 0; count < TMP_MAX; value += 7777, ++count)
+    {
+      uint64_t v = value;
+
+      /* Fill in the random bits.  */
+      XXXXXX[0] = letters[v % 62];
+      v /= 62;
+      XXXXXX[1] = letters[v % 62];
+      v /= 62;
+      XXXXXX[2] = letters[v % 62];
+      v /= 62;
+      XXXXXX[3] = letters[v % 62];
+      v /= 62;
+      XXXXXX[4] = letters[v % 62];
+      v /= 62;
+      XXXXXX[5] = letters[v % 62];
+
+      fd = __mkdir (tmpl, S_IRUSR | S_IWUSR | S_IXUSR);
+
+      if (fd >= 0)
+	{
+	  __set_errno (save_errno);
+	  return fd;
+	}
+      else if (errno != EEXIST)
+	return -1;
+    }
+
+  /* We got out of the loop because we ran out of combinations to try.  */
+  __set_errno (EEXIST);
+  return -1;
+}
+
+/* Generate a unique temporary directory from TEMPLATE.
+   The last six characters of TEMPLATE must be "XXXXXX";
+   they are replaced with a string that makes the filename unique.
+   The directory is created, mode 700, and its name is returned.
+   (This function comes from OpenBSD.) */
+char *
+mkdtemp (template)
+     char *template;
+{
+  if (gen_tempname (template))
+    return NULL;
+  else
+    return template;
+}
diff --git a/common/mkdtemp.h b/common/mkdtemp.h
new file mode 100644
index 00000000..dbc47ac3
--- /dev/null
+++ b/common/mkdtemp.h
@@ -0,0 +1,40 @@
+/* Creating a private temporary directory.
+   Copyright (C) 2001 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software Foundation,
+   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
+
+#ifndef PARAMS
+# if __STDC__ || defined __GNUC__ || defined __SUNPRO_C || defined __cplusplus || __PROTOTYPES
+#  define PARAMS(Args) Args
+# else
+#  define PARAMS(Args) ()
+# endif
+#endif
+
+#if HAVE_MKDTEMP
+
+/* Get mkdtemp() declaration.  */
+#include <stdlib.h>
+
+#else
+
+/* Create a unique temporary directory from TEMPLATE.
+   The last six characters of TEMPLATE must be "XXXXXX";
+   they are replaced with a string that makes the directory name unique.
+   Returns TEMPLATE, or a null pointer if it cannot get a unique name.
+   The directory is created mode 700.  */
+extern char * mkdtemp PARAMS ((char *template));
+
+#endif
diff --git a/configure.ac b/configure.ac
index 3ab9e35f..1c03d1a9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -313,6 +313,15 @@ AC_CHECK_FUNC(inet_aton,,[
 	      AC_CHECK_LIB(resolv,inet_aton, [
 			   EXTRA_CHOOSER_LIBS="$EXTRA_CHOOSER_LIBS -lresolv"])])
 
+dnl ---------------------------------------------------------------------------
+dnl - Check for mkdtemp
+dnl ---------------------------------------------------------------------------
+
+mkdtemp_missing=false
+AC_CHECK_FUNC(mkdtemp,
+    [AC_DEFINE([HAVE_MKDTEMP], 1, [Have GlibC function to make temp dirs])],
+    mkdtemp_missing=true)
+AM_CONDITIONAL(MKDTEMP_MISSING, test x$mkdtemp_missing = xtrue)
 
 dnl ---------------------------------------------------------------------------
 dnl - Check for IPv6
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);
         }