[PATCH 1/2] posix: Sync tempname with gnulib [BZ #26648]

[PATCH 1/2] posix: Sync tempname with gnulib [BZ #26648]

[Adhemerval Zanella]

It syncs with gnulib commit ae9fb3d66 with the following change:

--- glibc
+++ gnulib
@@ -61,7 +61,7 @@
 # define __gen_tempname gen_tempname
 # define __mkdir mkdir
 # define __open open
-# define __lstat64(version, file, buf) lstat (file, buf)
+# define __lxstat64(version, file, buf) lstat (file, buf)
 # define __getrandom getrandom
 # define __clock_gettime64 clock_gettime
 # define __timespec64 timespec
@@ -96,7 +96,7 @@ static int
 direxists (const char *dir)
 {
   struct_stat64 buf;
-  return __stat64 (dir, &buf) == 0 && S_ISDIR (buf.st_mode);
+  return __xstat64 (_STAT_VER, dir, &buf) == 0 && S_ISDIR (buf.st_mode);
 }

 /* Path search algorithm, for tmpnam, tmpfile, etc.  If DIR is
@@ -188,7 +188,7 @@ try_nocreate (char *tmpl, void *flags _G
 {
   struct_stat64 st;

-  if (__lstat64 (tmpl, &st) == 0 || errno == EOVERFLOW)
+  if (__lxstat64 (_STAT_VER, tmpl, &st) == 0 || errno == EOVERFLOW)
     __set_errno (EEXIST);
   return errno == ENOENT ? 0 : -1;
 }

The implementation now uses getrandom on each iteration to get entropy
and only uses the clock as source of entropy if getrandom fails.

Checked on x86_64-linux-gnu.
---
 sysdeps/posix/tempname.c | 278 +++++++++++++++++++++++----------------
 1 file changed, 162 insertions(+), 116 deletions(-)

diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c
index c98141e8d1..193d791103 100644
--- a/sysdeps/posix/tempname.c
+++ b/sysdeps/posix/tempname.c
@@ -16,7 +16,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #if !_LIBC
-# include <config.h>
+# include <libc-config.h>
 # include "tempname.h"
 #endif
 
@@ -24,9 +24,6 @@
 #include <assert.h>
 
 #include <errno.h>
-#ifndef __set_errno
-# define __set_errno(Val) errno = (Val)
-#endif
 
 #include <stdio.h>
 #ifndef P_tmpdir
@@ -36,12 +33,12 @@
 # define TMP_MAX 238328
 #endif
 #ifndef __GT_FILE
-# define __GT_FILE	0
-# define __GT_DIR	1
-# define __GT_NOCREATE	2
+# define __GT_FILE      0
+# define __GT_DIR       1
+# define __GT_NOCREATE  2
 #endif
-#if !_LIBC && (GT_FILE != __GT_FILE || GT_DIR != __GT_DIR	\
-	       || GT_NOCREATE != __GT_NOCREATE)
+#if !_LIBC && (GT_FILE != __GT_FILE || GT_DIR != __GT_DIR       \
+               || GT_NOCREATE != __GT_NOCREATE)
 # error report this to bug-gnulib@gnu.org
 #endif
 
@@ -50,11 +47,11 @@
 #include <string.h>
 
 #include <fcntl.h>
-#include <time.h>
+#include <stdalign.h>
 #include <stdint.h>
-#include <unistd.h>
-
+#include <sys/random.h>
 #include <sys/stat.h>
+#include <time.h>
 
 #if _LIBC
 # define struct_stat64 struct stat64
@@ -62,32 +59,36 @@
 #else
 # define struct_stat64 struct stat
 # define __gen_tempname gen_tempname
-# define __getpid getpid
 # define __mkdir mkdir
 # define __open open
-# define __secure_getenv secure_getenv
+# define __lstat64(version, file, buf) lstat (file, buf)
+# define __getrandom getrandom
+# define __clock_gettime64 clock_gettime
+# define __timespec64 timespec
 #endif
 
-#ifdef _LIBC
-# include <random-bits.h>
-# define RANDOM_BITS(Var) ((Var) = random_bits ())
-# else
-# define RANDOM_BITS(Var) \
-    {                                                                         \
-      struct timespec ts;                                                     \
-      clock_gettime (CLOCK_REALTIME, &ts);                                    \
-      (Var) = ((uint64_t) tv.tv_nsec << 16) ^ tv.tv_sec;                      \
-    }
-#endif
+/* Use getrandom if it works, falling back on a 64-bit linear
+   congruential generator that starts with Var's value
+   mixed in with a clock's low-order bits if available.  */
+typedef uint_fast64_t random_value;
+#define RANDOM_VALUE_MAX UINT_FAST64_MAX
+#define BASE_62_DIGITS 10 /* 62**10 < UINT_FAST64_MAX */
+#define BASE_62_POWER (62LL * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62)
 
-/* 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
+static random_value
+random_bits (random_value var)
+{
+  random_value r;
+  if (__getrandom (&r, sizeof r, 0) == sizeof r)
+    return r;
+#if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
+  /* Add entropy if getrandom is not supported.  */
+  struct __timespec64 tv;
+  __clock_gettime64 (CLOCK_MONOTONIC, &tv);
+  var ^= tv.tv_nsec;
 #endif
+  return 2862933555777941757 * var + 3037000493;
+}
 
 #if _LIBC
 /* Return nonzero if DIR is an existent directory.  */
@@ -106,7 +107,7 @@ direxists (const char *dir)
    enough space in TMPL. */
 int
 __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx,
-	       int try_tmpdir)
+               int try_tmpdir)
 {
   const char *d;
   size_t dlen, plen;
@@ -120,35 +121,35 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx,
     {
       plen = strlen (pfx);
       if (plen > 5)
-	plen = 5;
+        plen = 5;
     }
 
   if (try_tmpdir)
     {
       d = __secure_getenv ("TMPDIR");
       if (d != NULL && direxists (d))
-	dir = d;
+        dir = d;
       else if (dir != NULL && direxists (dir))
-	/* nothing */ ;
+        /* nothing */ ;
       else
-	dir = NULL;
+        dir = NULL;
     }
   if (dir == NULL)
     {
       if (direxists (P_tmpdir))
-	dir = P_tmpdir;
+        dir = P_tmpdir;
       else if (strcmp (P_tmpdir, "/tmp") != 0 && direxists ("/tmp"))
-	dir = "/tmp";
+        dir = "/tmp";
       else
-	{
-	  __set_errno (ENOENT);
-	  return -1;
-	}
+        {
+          __set_errno (ENOENT);
+          return -1;
+        }
     }
 
   dlen = strlen (dir);
   while (dlen > 1 && dir[dlen - 1] == '/')
-    dlen--;			/* remove trailing slashes */
+    dlen--;                     /* remove trailing slashes */
 
   /* check we have room for "${dir}/${pfx}XXXXXX\0" */
   if (tmpl_len < dlen + 1 + plen + 6 + 1)
@@ -162,39 +163,91 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx,
 }
 #endif /* _LIBC */
 
+#if _LIBC
+static int try_tempname_len (char *, int, void *, int (*) (char *, void *),
+                             size_t);
+#endif
+
+static int
+try_file (char *tmpl, void *flags)
+{
+  int *openflags = flags;
+  return __open (tmpl,
+                 (*openflags & ~O_ACCMODE)
+                 | O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR);
+}
+
+static int
+try_dir (char *tmpl, void *flags _GL_UNUSED)
+{
+  return __mkdir (tmpl, S_IRUSR | S_IWUSR | S_IXUSR);
+}
+
+static int
+try_nocreate (char *tmpl, void *flags _GL_UNUSED)
+{
+  struct_stat64 st;
+
+  if (__lstat64 (tmpl, &st) == 0 || errno == EOVERFLOW)
+    __set_errno (EEXIST);
+  return errno == ENOENT ? 0 : -1;
+}
+
 /* These are the characters used in temporary file names.  */
 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", possibly with a suffix).
+   rules for mk[s]temp (i.e., end in at least X_SUFFIX_LEN "X"s,
+   possibly with a suffix).
    The name constructed does not exist at the time of the call to
-   __gen_tempname.  TMPL is overwritten with the result.
+   this function.  TMPL is overwritten with the result.
 
    KIND may be one of:
-   __GT_NOCREATE:	simply verify that the name does not exist
-			at the time of the call.
-   __GT_FILE:		create the file using open(O_CREAT|O_EXCL)
-			and return a read-write fd.  The file is mode 0600.
-   __GT_DIR:		create a directory, which will be mode 0700.
+   __GT_NOCREATE:       simply verify that the name does not exist
+                        at the time of the call.
+   __GT_FILE:           create the file using open(O_CREAT|O_EXCL)
+                        and return a read-write fd.  The file is mode 0600.
+   __GT_DIR:            create a directory, which will be mode 0700.
 
    We use a clever algorithm to get hard-to-predict names. */
+#ifdef _LIBC
+static
+#endif
 int
-__gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
+gen_tempname_len (char *tmpl, int suffixlen, int flags, int kind,
+                  size_t x_suffix_len)
 {
-  int len;
+  static int (*const tryfunc[]) (char *, void *) =
+    {
+      [__GT_FILE] = try_file,
+      [__GT_DIR] = try_dir,
+      [__GT_NOCREATE] = try_nocreate
+    };
+  return try_tempname_len (tmpl, suffixlen, &flags, tryfunc[kind],
+                           x_suffix_len);
+}
+
+#ifdef _LIBC
+static
+#endif
+int
+try_tempname_len (char *tmpl, int suffixlen, void *args,
+                  int (*tryfunc) (char *, void *), size_t x_suffix_len)
+{
+  size_t len;
   char *XXXXXX;
   unsigned int count;
   int fd = -1;
   int save_errno = errno;
-  struct_stat64 st;
 
   /* A lower bound on the number of temporary files to attempt to
      generate.  The maximum total number of temporary file names that
      can exist for a given template is 62**6.  It should never be
      necessary to try all of these combinations.  Instead if a reasonable
      number of names is tried (we define reasonable as 62**3) fail to
-     give the system administrator the chance to remove the problems.  */
+     give the system administrator the chance to remove the problems.
+     This value requires that X_SUFFIX_LEN be at least 3.  */
 #define ATTEMPTS_MIN (62 * 62 * 62)
 
   /* The number of times to attempt to generate a temporary file.  To
@@ -205,82 +258,75 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
   unsigned int attempts = ATTEMPTS_MIN;
 #endif
 
+  /* A random variable.  The initial value is used only the for fallback path
+     on 'random_bits' on 'getrandom' failure.  Its initial value tries to use
+     some entropy from the ASLR and ignore possible bits from the stack
+     alignment.  */
+  random_value v = ((uintptr_t) &v) / alignof (max_align_t);
+
+  /* How many random base-62 digits can currently be extracted from V.  */
+  int vdigits = 0;
+
+  /* Least unfair value for V.  If V is less than this, V can generate
+     BASE_62_DIGITS digits fairly.  Otherwise it might be biased.  */
+  random_value const unfair_min
+    = RANDOM_VALUE_MAX - RANDOM_VALUE_MAX % BASE_62_POWER;
+
   len = strlen (tmpl);
-  if (len < 6 + suffixlen || memcmp (&tmpl[len - 6 - suffixlen], "XXXXXX", 6))
+  if (len < x_suffix_len + suffixlen
+      || strspn (&tmpl[len - x_suffix_len - suffixlen], "X") < x_suffix_len)
     {
       __set_errno (EINVAL);
       return -1;
     }
 
   /* This is where the Xs start.  */
-  XXXXXX = &tmpl[len - 6 - suffixlen];
+  XXXXXX = &tmpl[len - x_suffix_len - suffixlen];
 
-  uint64_t pid = (uint64_t) __getpid () << 32;
   for (count = 0; count < attempts; ++count)
     {
-      uint64_t v;
-      /* Get some more or less random data.  */
-      RANDOM_BITS (v);
-      v ^= pid;
-
-      /* 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];
-
-      switch (kind)
-	{
-	case __GT_FILE:
-	  fd = __open (tmpl,
-		       (flags & ~O_ACCMODE)
-		       | O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR);
-	  break;
-
-	case __GT_DIR:
-	  fd = __mkdir (tmpl, S_IRUSR | S_IWUSR | S_IXUSR);
-	  break;
-
-	case __GT_NOCREATE:
-	  /* This case is backward from the other three.  __gen_tempname
-	     succeeds if lstat fails because the name does not exist.
-	     Note the continue to bypass the common logic at the bottom
-	     of the loop.  */
-	  if (__lstat64 (tmpl, &st) < 0)
-	    {
-	      if (errno == ENOENT)
-		{
-		  __set_errno (save_errno);
-		  return 0;
-		}
-	      else
-		/* Give up now. */
-		return -1;
-	    }
-	  continue;
-
-	default:
-	  assert (! "invalid KIND in __gen_tempname");
-	  abort ();
-	}
-
+      for (size_t i = 0; i < x_suffix_len; i++)
+        {
+          if (vdigits == 0)
+            {
+              do
+                v = random_bits (v);
+              while (unfair_min <= v);
+
+              vdigits = BASE_62_DIGITS;
+            }
+
+          XXXXXX[i] = letters[v % 62];
+          v /= 62;
+          vdigits--;
+        }
+
+      fd = tryfunc (tmpl, args);
       if (fd >= 0)
-	{
-	  __set_errno (save_errno);
-	  return fd;
-	}
+        {
+          __set_errno (save_errno);
+          return fd;
+        }
       else if (errno != EEXIST)
-	return -1;
+        return -1;
     }
 
   /* We got out of the loop because we ran out of combinations to try.  */
   __set_errno (EEXIST);
   return -1;
 }
+
+int
+__gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
+{
+  return gen_tempname_len (tmpl, suffixlen, flags, kind, 6);
+}
+
+#if !_LIBC
+int
+try_tempname (char *tmpl, int suffixlen, void *args,
+              int (*tryfunc) (char *, void *))
+{
+  return try_tempname_len (tmpl, suffixlen, args, tryfunc, 6);
+}
+#endif
-- 
2.25.1

[PATCH 2/2] posix: Improve randomness on try_tempname_len

[Adhemerval Zanella]

For __GT_NOCREATE (mktemp, tempnam, tmpnam) getrandom is also used
on first try, otherwise randomness is obtained using the clock plus
a linear congruential generator.

Also for getrandom GRND_NONBLOCK is used to avoid blocking indefinitely
on some older kernels.

Checked on x86_64-linux-gnu.
---
 sysdeps/posix/tempname.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c
index 193d791103..06db694181 100644
--- a/sysdeps/posix/tempname.c
+++ b/sysdeps/posix/tempname.c
@@ -76,10 +76,11 @@ typedef uint_fast64_t random_value;
 #define BASE_62_POWER (62LL * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62)
 
 static random_value
-random_bits (random_value var)
+random_bits (random_value var, bool use_getrandom)
 {
   random_value r;
-  if (__getrandom (&r, sizeof r, 0) == sizeof r)
+  /* Without GRND_NONBLOCK it can be blocked for minutes on some systems.  */
+  if (use_getrandom && __getrandom (&r, sizeof r, GRND_NONBLOCK) == sizeof r)
     return r;
 #if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
   /* Add entropy if getrandom is not supported.  */
@@ -263,9 +264,10 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
      some entropy from the ASLR and ignore possible bits from the stack
      alignment.  */
   random_value v = ((uintptr_t) &v) / alignof (max_align_t);
+  v = random_bits (v, tryfunc == try_nocreate);
 
   /* How many random base-62 digits can currently be extracted from V.  */
-  int vdigits = 0;
+  int vdigits = BASE_62_DIGITS;
 
   /* Least unfair value for V.  If V is less than this, V can generate
      BASE_62_DIGITS digits fairly.  Otherwise it might be biased.  */
@@ -290,7 +292,7 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
           if (vdigits == 0)
             {
               do
-                v = random_bits (v);
+                v = random_bits (v, true);
               while (unfair_min <= v);
 
               vdigits = BASE_62_DIGITS;
-- 
2.25.1

Re: [PATCH 1/2] posix: Sync tempname with gnulib [BZ #26648]

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On 1/4/21 9:03 AM, Adhemerval Zanella wrote:

> -# define __lstat64(version, file, buf) lstat (file, buf)
> +# define __lxstat64(version, file, buf) lstat (file, buf)

That change isn't quite right for the !_LIBC case, since it doesn't 
define the __stat64 macro and it defines __lstat64 with the wrong API.

I installed the attached patch into Gnulib, which should do the change 
right for both Gnulib and glibc, the idea being that the Gnulib and 
glibc source files can be identical.

[-- Attachment #2: 0001-tempname-sync-with-proposed-glibc-patch.patch --]
[-- Type: text/x-patch, Size: 2252 bytes --]

From 8eb0e08598b9815f0e85ce1bde676169920087ef Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 8 Jan 2021 17:31:28 -0800
Subject: [PATCH] tempname: sync with proposed glibc patch

This is from Adhemerval Zanella in:
https://sourceware.org/pipermail/libc-alpha/2021-January/121301.html
* lib/tempname.c (__lxstat64): Remove.
(__lstat64, __stat64): New replacement macros.  All uses changed.
---
 ChangeLog      | 6 ++++++
 lib/tempname.c | 7 ++++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a2787f59a..812888f8e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2021-01-08  Paul Eggert  <eggert@cs.ucla.edu>
 
+	tempname: sync with proposed glibc patch
+	This is from Adhemerval Zanella in:
+	https://sourceware.org/pipermail/libc-alpha/2021-January/121301.html
+	* lib/tempname.c (__lxstat64): Remove.
+	(__lstat64, __stat64): New replacement macros.  All uses changed.
+
 	regex: stop using alloca
 	* lib/regex_internal.h: Do not include <alloca.h> or define
 	__libc_use_alloca or alloca.  Patch written by Adhemerval Zanella:
diff --git a/lib/tempname.c b/lib/tempname.c
index 9ed67fec2..f196b9862 100644
--- a/lib/tempname.c
+++ b/lib/tempname.c
@@ -61,7 +61,8 @@
 # define __gen_tempname gen_tempname
 # define __mkdir mkdir
 # define __open open
-# define __lxstat64(version, file, buf) lstat (file, buf)
+# define __lstat64(file, buf) lstat (file, buf)
+# define __stat64(file, buf) stat (file, buf)
 # define __getrandom getrandom
 # define __clock_gettime64 clock_gettime
 # define __timespec64 timespec
@@ -96,7 +97,7 @@ static int
 direxists (const char *dir)
 {
   struct_stat64 buf;
-  return __xstat64 (_STAT_VER, dir, &buf) == 0 && S_ISDIR (buf.st_mode);
+  return __stat64 (dir, &buf) == 0 && S_ISDIR (buf.st_mode);
 }
 
 /* Path search algorithm, for tmpnam, tmpfile, etc.  If DIR is
@@ -188,7 +189,7 @@ try_nocreate (char *tmpl, void *flags _GL_UNUSED)
 {
   struct_stat64 st;
 
-  if (__lxstat64 (_STAT_VER, tmpl, &st) == 0 || errno == EOVERFLOW)
+  if (__lstat64 (tmpl, &st) == 0 || errno == EOVERFLOW)
     __set_errno (EEXIST);
   return errno == ENOENT ? 0 : -1;
 }
-- 
2.27.0

Re: [PATCH 2/2] posix: Improve randomness on try_tempname_len

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On 1/4/21 9:03 AM, Adhemerval Zanella wrote:
> For __GT_NOCREATE (mktemp, tempnam, tmpnam) getrandom is also used
> on first try, otherwise randomness is obtained using the clock plus
> a linear congruential generator.

Why not use getrandom in the first try also for __GT_DIR (mkdtemp) and 
__GT_FILE (mkostemp, mkostemps, mkstemp, mkstemps, tmpfile)? That is 
what Gnulib tempname.c is doing now. This not only simplifies the code, 
it improves resistance to some (admittedly less-likely) attacks.

> Also for getrandom GRND_NONBLOCK is used to avoid blocking indefinitely
> on some older kernels.

Thanks, I installed that part of the proposal into Gnulib by installing 
the attached. The idea is for tempname.c to be identical after we get 
the abovementioned issue worked out.

[-- Attachment #2: 0001-tempname-don-t-block-for-minutes.patch --]
[-- Type: text/x-patch, Size: 1899 bytes --]

From b0ebaf83a49fe4a895a78ddf5b0c4a029e34c566 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 8 Jan 2021 17:54:30 -0800
Subject: [PATCH] =?UTF-8?q?tempname:=20don=E2=80=99t=20block=20for=20minut?=
 =?UTF-8?q?es?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Derived from a patch proposed by Adhemerval Zanella in:
https://sourceware.org/pipermail/libc-alpha/2021-January/121302.html
* lib/tempname.c (random_bits): Use GRND_NONBLOCK.
---
 ChangeLog      | 5 +++++
 lib/tempname.c | 5 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 812888f8e..b76330e5b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2021-01-08  Paul Eggert  <eggert@cs.ucla.edu>
 
+	tempname: don’t block for minutes
+	Derived from a patch proposed by Adhemerval Zanella in:
+	https://sourceware.org/pipermail/libc-alpha/2021-January/121302.html
+	* lib/tempname.c (random_bits): Use GRND_NONBLOCK.
+
 	tempname: sync with proposed glibc patch
 	This is from Adhemerval Zanella in:
 	https://sourceware.org/pipermail/libc-alpha/2021-January/121301.html
diff --git a/lib/tempname.c b/lib/tempname.c
index f196b9862..f199b25a7 100644
--- a/lib/tempname.c
+++ b/lib/tempname.c
@@ -80,10 +80,11 @@ static random_value
 random_bits (random_value var)
 {
   random_value r;
-  if (__getrandom (&r, sizeof r, 0) == sizeof r)
+  /* Without GRND_NONBLOCK it can be blocked for minutes on some systems.  */
+  if (__getrandom (&r, sizeof r, GRND_NONBLOCK) == sizeof r)
     return r;
 #if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
-  /* Add entropy if getrandom is not supported.  */
+  /* Add entropy if getrandom did not work.  */
   struct __timespec64 tv;
   __clock_gettime64 (CLOCK_MONOTONIC, &tv);
   var ^= tv.tv_nsec;
-- 
2.27.0

Re: [PATCH 2/2] posix: Improve randomness on try_tempname_len

[Adhemerval Zanella]

On 08/01/2021 23:20, Paul Eggert wrote:
> On 1/4/21 9:03 AM, Adhemerval Zanella wrote:
>> For __GT_NOCREATE (mktemp, tempnam, tmpnam) getrandom is also used
>> on first try, otherwise randomness is obtained using the clock plus
>> a linear congruential generator.
> 
> Why not use getrandom in the first try also for __GT_DIR (mkdtemp) and __GT_FILE (mkostemp, mkostemps, mkstemp, mkstemps, tmpfile)? That is what Gnulib tempname.c is doing now. This not only simplifies the code, it improves resistance to some (admittedly less-likely) attacks.

The idea is to always issue getrandom for __GT_DIR or __GT_FILE on first try,
as you suggested initially [1].  I followed your idea [2]:

  Here's an idea: use getrandom in the first try only for the __GT_NOCREATE case. 
  Although a bit more complicated, I expect this would address both your entropy 
  and my security concerns.

The current code should address Jakub concerns of using getrandom without 
GRND_NONBLOCK and not using on on first try (to avoid deplete the random 
entropy pool) and use getrandom only when a collision if found. I will merge
the code, close the bug, and we can work whether use getrandom only for
__GT_DIR/__GT_FILE is an improvement or not.

> 
>> Also for getrandom GRND_NONBLOCK is used to avoid blocking indefinitely
>> on some older kernels.
> 
> Thanks, I installed that part of the proposal into Gnulib by installing the attached. The idea is for tempname.c to be identical after we get the abovementioned issue worked out.

[1] https://sourceware.org/pipermail/libc-alpha/2020-September/117535.html
[2] https://sourceware.org/pipermail/libc-alpha/2020-September/117539.html

Re: [PATCH 1/2] posix: Sync tempname with gnulib [BZ #26648]

[Adhemerval Zanella]

On 08/01/2021 22:58, Paul Eggert wrote:
> On 1/4/21 9:03 AM, Adhemerval Zanella wrote:
> 
>> -# define __lstat64(version, file, buf) lstat (file, buf)
>> +# define __lxstat64(version, file, buf) lstat (file, buf)
> 
> That change isn't quite right for the !_LIBC case, since it doesn't define the __stat64 macro and it defines __lstat64 with the wrong API.
> 
> I installed the attached patch into Gnulib, which should do the change right for both Gnulib and glibc, the idea being that the Gnulib and glibc source files can be identical.

Indeed, I forgot that these symbols are still provided internally.

Re: [PATCH 2/2] posix: Improve randomness on try_tempname_len

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 838 bytes --]

On 1/11/21 4:29 AM, Adhemerval Zanella wrote:

> The idea is to always issue getrandom for __GT_DIR or __GT_FILE on first try,
> as you suggested initially [1].  I followed your idea [2]:... > [1] 
https://sourceware.org/pipermail/libc-alpha/2020-September/117535.html
> [2] https://sourceware.org/pipermail/libc-alpha/2020-September/117539.html

Ah, thanks, I'd forgotten about that conversation.

I looked at the patch 
<https://sourceware.org/pipermail/libc-alpha/2021-January/121302.html> 
again. A couple of small things. First, it uses bool so needs to include 
stdbool.h. Second, the generated code's a bit smaller if we call 
random_bits only once. I added those two changes and installed the 
attached patch to Gnulib master on savannah, with the idea being that 
Gnulib's tempname.c can be identical to glibc's.

[-- Attachment #2: 0001-tempname-consume-less-entropy.patch --]
[-- Type: text/x-patch, Size: 3585 bytes --]

From 23c0672c281a949c254ef0c173eab987ab876e29 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 11 Jan 2021 16:46:12 -0800
Subject: [PATCH] tempname: consume less entropy

Derived from a glibc patch proposed by Adhemerval Zanella in:
https://sourceware.org/pipermail/libc-alpha/2021-January/121302.html
* lib/tempname.c: Include stdbool.h.
(random_bits): New arg use_getrandom.
(try_tempname_len): Skip getrandom on the first try,
unless __GT_NOCREATE.
* modules/tempname (Depends-on): Add stdbool.
---
 ChangeLog        | 11 +++++++++++
 lib/tempname.c   | 17 ++++++++++++++---
 modules/tempname |  1 +
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c6f5295b4..0d0144242 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2021-01-11  Paul Eggert  <eggert@cs.ucla.edu>
+
+	tempname: consume less entropy
+	Derived from a glibc patch proposed by Adhemerval Zanella in:
+	https://sourceware.org/pipermail/libc-alpha/2021-January/121302.html
+	* lib/tempname.c: Include stdbool.h.
+	(random_bits): New arg use_getrandom.
+	(try_tempname_len): Skip getrandom on the first try,
+	unless __GT_NOCREATE.
+	* modules/tempname (Depends-on): Add stdbool.
+
 2021-01-10  Bruno Haible  <bruno@clisp.org>
 
 	lchmod-tests: Fix link error.
diff --git a/lib/tempname.c b/lib/tempname.c
index f199b25a7..5f804b38d 100644
--- a/lib/tempname.c
+++ b/lib/tempname.c
@@ -22,6 +22,7 @@
 
 #include <sys/types.h>
 #include <assert.h>
+#include <stdbool.h>
 
 #include <errno.h>
 
@@ -77,11 +78,11 @@ typedef uint_fast64_t random_value;
 #define BASE_62_POWER (62LL * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62)
 
 static random_value
-random_bits (random_value var)
+random_bits (random_value var, bool use_getrandom)
 {
   random_value r;
   /* Without GRND_NONBLOCK it can be blocked for minutes on some systems.  */
-  if (__getrandom (&r, sizeof r, GRND_NONBLOCK) == sizeof r)
+  if (use_getrandom && __getrandom (&r, sizeof r, GRND_NONBLOCK) == sizeof r)
     return r;
 #if _LIBC || (defined CLOCK_MONOTONIC && HAVE_CLOCK_GETTIME)
   /* Add entropy if getrandom did not work.  */
@@ -269,6 +270,13 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
   /* How many random base-62 digits can currently be extracted from V.  */
   int vdigits = 0;
 
+  /* Whether to consume entropy when acquiring random bits.  On the
+     first try it's worth the entropy cost with __GT_NOCREATE, which
+     is inherently insecure and can use the entropy to make it a bit
+     less secure.  On the (rare) second and later attempts it might
+     help against DoS attacks.  */
+  bool use_getrandom = tryfunc == try_nocreate;
+
   /* Least unfair value for V.  If V is less than this, V can generate
      BASE_62_DIGITS digits fairly.  Otherwise it might be biased.  */
   random_value const unfair_min
@@ -292,7 +300,10 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
           if (vdigits == 0)
             {
               do
-                v = random_bits (v);
+                {
+                  v = random_bits (v, use_getrandom);
+                  use_getrandom = true;
+                }
               while (unfair_min <= v);
 
               vdigits = BASE_62_DIGITS;
diff --git a/modules/tempname b/modules/tempname
index 27b0d3d23..4779735d9 100644
--- a/modules/tempname
+++ b/modules/tempname
@@ -17,6 +17,7 @@ libc-config
 lstat
 mkdir
 stdalign
+stdbool
 stdint
 sys_stat
 time
-- 
2.27.0