* [PATCH] posix: Fix __gen_tempname iteration entropy (BZ#15813)
@ 2019-04-04 8:31 adhemerval.zanella
2019-04-05 1:44 ` Paul Eggert
2019-04-29 10:46 ` Yann Droneaud
0 siblings, 2 replies; 4+ messages in thread
From: adhemerval.zanella @ 2019-04-04 8:31 UTC (permalink / raw)
To: libc-alpha; +Cc: Adhemerval Zanella
From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Patch "Do not use HP_TIMING_NOW for random bits (359653aaacad4)" fixed
mostly of the __gen_tempname issues described by BZ#15813. This patch
fixes the remaining one by adding a extra call to random_bits for
eac iteration while trying to create the random name.
The patch also cleanups the tempname implementation since now it
deviates from gnulib counterpart.
Checked on powerpc64le-linux-gnu.
[BZ #15813]
* sysdeps/posix/tempname.c: Remove ununsed includes, redundant
definitions, and defines used only for gnulib.
(__gen_tempname): Set number of attemps to TMP_MAX and use
random_bits on eachh iteration.
---
ChangeLog | 8 ++++
sysdeps/posix/tempname.c | 97 ++++------------------------------------
2 files changed, 17 insertions(+), 88 deletions(-)
diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c
index de346949b2..ccbab46029 100644
--- a/sysdeps/posix/tempname.c
+++ b/sysdeps/posix/tempname.c
@@ -15,88 +15,18 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#if !_LIBC
-# include <config.h>
-# include "tempname.h"
-#endif
-
-#include <sys/types.h>
-#include <assert.h>
-
-#include <errno.h>
-#ifndef __set_errno
-# define __set_errno(Val) errno = (Val)
-#endif
-
#include <stdio.h>
-#ifndef P_tmpdir
-# define P_tmpdir "/tmp"
-#endif
-#ifndef TMP_MAX
-# define TMP_MAX 238328
-#endif
-#ifndef __GT_FILE
-# 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)
-# error report this to bug-gnulib@gnu.org
-#endif
-
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <fcntl.h>
-#include <sys/time.h>
-#include <stdint.h>
#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <assert.h>
+#include <random-bits.h>
-#include <sys/stat.h>
-
-#if _LIBC
-# define struct_stat64 struct stat64
-# define __secure_getenv __libc_secure_getenv
-#else
-# define struct_stat64 struct stat
-# define __gen_tempname gen_tempname
-# define __getpid getpid
-# define __gettimeofday gettimeofday
-# define __mkdir mkdir
-# define __open open
-# define __lxstat64(version, file, buf) lstat (file, buf)
-# define __secure_getenv secure_getenv
-#endif
-
-#ifdef _LIBC
-# include <random-bits.h>
-# define RANDOM_BITS(Var) ((Var) = random_bits ())
-# else
-# define RANDOM_BITS(Var) \
- { \
- struct timeval tv; \
- __gettimeofday (&tv, NULL); \
- (Var) = ((uint64_t) tv.tv_usec << 16) ^ tv.tv_sec; \
- }
-#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
-
-#if _LIBC
/* Return nonzero if DIR is an existent directory. */
static int
direxists (const char *dir)
{
- struct_stat64 buf;
+ struct stat64 buf;
return __xstat64 (_STAT_VER, dir, &buf) == 0 && S_ISDIR (buf.st_mode);
}
@@ -127,7 +57,7 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx,
if (try_tmpdir)
{
- d = __secure_getenv ("TMPDIR");
+ d = __libc_secure_getenv ("TMPDIR");
if (d != NULL && direxists (d))
dir = d;
else if (dir != NULL && direxists (dir))
@@ -162,7 +92,6 @@ __path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx,
sprintf (tmpl, "%.*s/%.*sXXXXXX", (int) dlen, dir, (int) plen, pfx);
return 0;
}
-#endif /* _LIBC */
/* These are the characters used in temporary file names. */
static const char letters[] =
@@ -190,7 +119,7 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
unsigned int count;
int fd = -1;
int save_errno = errno;
- struct_stat64 st;
+ 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
@@ -198,15 +127,7 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
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. */
-#define ATTEMPTS_MIN (62 * 62 * 62)
-
- /* The number of times to attempt to generate a temporary file. To
- conform to POSIX, this must be no smaller than TMP_MAX. */
-#if ATTEMPTS_MIN < TMP_MAX
unsigned int attempts = TMP_MAX;
-#else
- unsigned int attempts = ATTEMPTS_MIN;
-#endif
len = strlen (tmpl);
if (len < 6 + suffixlen || memcmp (&tmpl[len - 6 - suffixlen], "XXXXXX", 6))
@@ -219,10 +140,10 @@ __gen_tempname (char *tmpl, int suffixlen, int flags, int kind)
XXXXXX = &tmpl[len - 6 - suffixlen];
/* Get some more or less random data. */
- RANDOM_BITS (value);
+ value = random_bits ();
value ^= (uint64_t)__getpid () << 32;
- for (count = 0; count < attempts; value += 7777, ++count)
+ for (count = 0; count < attempts; value += random_bits (), ++count)
{
uint64_t v = value;
--
2.19.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] posix: Fix __gen_tempname iteration entropy (BZ#15813)
2019-04-04 8:31 [PATCH] posix: Fix __gen_tempname iteration entropy (BZ#15813) adhemerval.zanella
@ 2019-04-05 1:44 ` Paul Eggert
2019-04-09 12:11 ` Adhemerval Zanella
2019-04-29 10:46 ` Yann Droneaud
1 sibling, 1 reply; 4+ messages in thread
From: Paul Eggert @ 2019-04-05 1:44 UTC (permalink / raw)
To: adhemerval.zanella, libc-alpha
On 4/4/19 1:31 AM, adhemerval.zanella@linaro.org wrote:
> The patch also cleanups the tempname implementation since now it
> deviates from gnulib counterpart.
Shouldn't we strive to keep them in sync? Does the Gnulib version need
the fix you're proposing for the glibc version?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] posix: Fix __gen_tempname iteration entropy (BZ#15813)
2019-04-05 1:44 ` Paul Eggert
@ 2019-04-09 12:11 ` Adhemerval Zanella
0 siblings, 0 replies; 4+ messages in thread
From: Adhemerval Zanella @ 2019-04-09 12:11 UTC (permalink / raw)
To: Paul Eggert, libc-alpha
On 04/04/2019 22:44, Paul Eggert wrote:
> On 4/4/19 1:31 AM, adhemerval.zanella@linaro.org wrote:
>> The patch also cleanups the tempname implementation since now it
>> deviates from gnulib counterpart.
>
> Shouldn't we strive to keep them in sync? Does the Gnulib version need
> the fix you're proposing for the glibc version?
>
I am not sure if gnulib is willing to add the random_bits implementation
based on clock_gettime, this patch is following Wilco idea to remove the
fallback gettimeofday and related code. Checking on gnulib code it seems
it already have a timespec module, so one can assume clock_gettime is
always supported (even if it calls gettimeofday in the end). I don't mind
sending a second version if gnulib adds the fix.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] posix: Fix __gen_tempname iteration entropy (BZ#15813)
2019-04-04 8:31 [PATCH] posix: Fix __gen_tempname iteration entropy (BZ#15813) adhemerval.zanella
2019-04-05 1:44 ` Paul Eggert
@ 2019-04-29 10:46 ` Yann Droneaud
1 sibling, 0 replies; 4+ messages in thread
From: Yann Droneaud @ 2019-04-29 10:46 UTC (permalink / raw)
To: adhemerval.zanella, libc-alpha
Le jeudi 04 avril 2019 Ã 15:31 +0700, adhemerval.zanella@linaro.org a
écrit :
> From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> Patch "Do not use HP_TIMING_NOW for random bits (359653aaacad4)"
> fixed
> mostly of the __gen_tempname issues described by BZ#15813. This patch
> fixes the remaining one by adding a extra call to random_bits for
> eac iteration while trying to create the random name.
>
> The patch also cleanups the tempname implementation since now it
> deviates from gnulib counterpart.
>
> Checked on powerpc64le-linux-gnu.
>
> [BZ #15813]
> * sysdeps/posix/tempname.c: Remove ununsed includes, redundant
> definitions, and defines used only for gnulib.
> (__gen_tempname): Set number of attemps to TMP_MAX and use
> random_bits on eachh iteration.
> ---
> ChangeLog | 8 ++++
> sysdeps/posix/tempname.c | 97 ++++--------------------------------
> ----
> 2 files changed, 17 insertions(+), 88 deletions(-)
>
For easier review (and maintenance) I would like this patch to be split
in 2 or more patches. At least: one for removing gnulib support, and
one to improve randomness for each retry.
Regards.
--
Yann Droneaud
OPTEYA
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-29 10:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-04 8:31 [PATCH] posix: Fix __gen_tempname iteration entropy (BZ#15813) adhemerval.zanella
2019-04-05 1:44 ` Paul Eggert
2019-04-09 12:11 ` Adhemerval Zanella
2019-04-29 10:46 ` Yann Droneaud
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).