* [PATCH 1/3] Merge _GL_UNUSED C23 patch from Gnulib
@ 2022-08-22 20:58 Paul Eggert
2022-08-22 20:58 ` [PATCH 2/3] Merge getopt " Paul Eggert
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Paul Eggert @ 2022-08-22 20:58 UTC (permalink / raw)
To: libc-alpha
* posix/getopt.c (_getopt_initialize):
* sysdeps/posix/tempname.c (try_dir, try_nocreate):
Put _GL_UNUSED before args instead of after.
This makes no difference for glibc.
It is needed for Gnulib when being compiled on
non-GCC C23 compilers.
---
posix/getopt.c | 4 ++--
sysdeps/posix/tempname.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/posix/getopt.c b/posix/getopt.c
index e9661c79fa..a160a4e3bd 100644
--- a/posix/getopt.c
+++ b/posix/getopt.c
@@ -377,8 +377,8 @@ process_long_option (int argc, char **argv, const char *optstring,
/* Initialize internal data upon the first call to getopt. */
static const char *
-_getopt_initialize (int argc _GL_UNUSED,
- char **argv _GL_UNUSED, const char *optstring,
+_getopt_initialize (_GL_UNUSED int argc,
+ _GL_UNUSED char **argv, const char *optstring,
struct _getopt_data *d, int posixly_correct)
{
/* Start processing options with ARGV-element 1 (since ARGV-element 0
diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c
index ebd2a43951..60f8541085 100644
--- a/sysdeps/posix/tempname.c
+++ b/sysdeps/posix/tempname.c
@@ -181,13 +181,13 @@ try_file (char *tmpl, void *flags)
}
static int
-try_dir (char *tmpl, void *flags _GL_UNUSED)
+try_dir (char *tmpl, _GL_UNUSED void *flags)
{
return __mkdir (tmpl, S_IRUSR | S_IWUSR | S_IXUSR);
}
static int
-try_nocreate (char *tmpl, void *flags _GL_UNUSED)
+try_nocreate (char *tmpl, _GL_UNUSED void *flags)
{
struct_stat64 st;
--
2.37.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] Merge getopt patch from Gnulib
2022-08-22 20:58 [PATCH 1/3] Merge _GL_UNUSED C23 patch from Gnulib Paul Eggert
@ 2022-08-22 20:58 ` Paul Eggert
2022-08-23 7:19 ` Florian Weimer
2022-08-22 20:58 ` [PATCH 3/3] Merge tempname ASLR etc. " Paul Eggert
2022-08-23 7:16 ` [PATCH 1/3] Merge _GL_UNUSED C23 " Florian Weimer
2 siblings, 1 reply; 7+ messages in thread
From: Paul Eggert @ 2022-08-22 20:58 UTC (permalink / raw)
To: libc-alpha
* posix/getopt.c [!_LIBC]: Merge _WIN32 patch from Gnulib
so that these source files are identical.
This makes no difference for glibc.
---
posix/getopt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/posix/getopt.c b/posix/getopt.c
index a160a4e3bd..128dc7fcf5 100644
--- a/posix/getopt.c
+++ b/posix/getopt.c
@@ -45,7 +45,8 @@
# define _(msgid) gettext (msgid)
/* When used standalone, flockfile and funlockfile might not be
available. */
-# ifndef _POSIX_THREAD_SAFE_FUNCTIONS
+# if (!defined _POSIX_THREAD_SAFE_FUNCTIONS \
+ || (defined _WIN32 && ! defined __CYGWIN__))
# define flockfile(fp) /* nop */
# define funlockfile(fp) /* nop */
# endif
--
2.37.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] Merge tempname ASLR etc. patch from Gnulib
2022-08-22 20:58 [PATCH 1/3] Merge _GL_UNUSED C23 patch from Gnulib Paul Eggert
2022-08-22 20:58 ` [PATCH 2/3] Merge getopt " Paul Eggert
@ 2022-08-22 20:58 ` Paul Eggert
2022-08-23 7:19 ` Florian Weimer
2022-08-23 7:16 ` [PATCH 1/3] Merge _GL_UNUSED C23 " Florian Weimer
2 siblings, 1 reply; 7+ messages in thread
From: Paul Eggert @ 2022-08-22 20:58 UTC (permalink / raw)
To: libc-alpha
Merge patch from Gnulib that fixes problems with ASLR info leak
and entropy loss. This syncs tempname.c with Gnulib commit
564b523fe97a8d63493aa68acb627b8c40744fb9 (2022-08-22) and
fixes <https://bugs.gnu.org/57129> which was reported against
Emacs and uncovered some unlikely Glibc bugs.
* sysdeps/posix/tempname.c: Don't include stdalign.h.
(mix_random_values): New function.
(random_bits): Use it. Args are now new value address and
old value, and this function now returns a success indicator.
Omit old USE_GETRANDOM argument: always try getrandom now, as
there is no good reason not to now that GRND_NONBLOCK is used.
Caller changed. Use CLOCK_REALTIME for for ersatz entropy,
as CLOCK_MONOTONIC doesn't work on some platforms.
Also, mix in ersatz entropy from tv_sec and from clock ().
(try_tempname_len): Do not mix in ASLR-based entropy, as
the result is published to the world and ASLR should be private.
Do not try to use a static var as that has issues if multithreaded.
Instead, simply generate new random bits.
Worry about bias only with high-quality random bits.
---
sysdeps/posix/tempname.c | 94 +++++++++++++++++++++++++---------------
1 file changed, 59 insertions(+), 35 deletions(-)
diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c
index 60f8541085..0e2f29f5de 100644
--- a/sysdeps/posix/tempname.c
+++ b/sysdeps/posix/tempname.c
@@ -48,7 +48,6 @@
#include <string.h>
#include <fcntl.h>
-#include <stdalign.h>
#include <stdint.h>
#include <sys/random.h>
#include <sys/stat.h>
@@ -77,20 +76,55 @@ typedef uint_fast64_t random_value;
#define BASE_62_DIGITS 10 /* 62**10 < UINT_FAST64_MAX */
#define BASE_62_POWER (62LL * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62 * 62)
+/* Return the result of mixing the entropy from R and S.
+ Assume that R and S are not particularly random,
+ and that the result should look randomish to an untrained eye. */
+
static random_value
-random_bits (random_value var, bool use_getrandom)
+mix_random_values (random_value r, random_value s)
+{
+ /* As this code is used only when high-quality randomness is neither
+ available nor necessary, there is no need for fancier polynomials
+ such as those in the Linux kernel's 'random' driver. */
+ return (2862933555777941757 * r + 3037000493) ^ s;
+}
+
+/* Set *R to a random value.
+ Return true if *R is set to high-quality value taken from getrandom.
+ Otherwise return false, falling back to a low-quality *R that might
+ depend on S.
+
+ This function returns false only when getrandom fails.
+ On GNU systems this should happen only early in the boot process,
+ when the fallback should be good enough for programs using tempname
+ because any attacker likely has root privileges already. */
+
+static bool
+random_bits (random_value *r, random_value s)
{
- random_value 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 did not work. */
+ if (__getrandom (r, sizeof *r, GRND_NONBLOCK) == sizeof *r)
+ return true;
+
+ /* If getrandom did not work, use ersatz entropy based on low-order
+ clock bits. On GNU systems getrandom should fail only
+ early in booting, when ersatz should be good enough.
+ Do not use ASLR-based entropy, as that would leak ASLR info into
+ the resulting file name which is typically public.
+
+ Of course we are in a state of sin here. */
+
+ random_value v = s;
+
+#if _LIBC || (defined CLOCK_REALTIME && HAVE_CLOCK_GETTIME)
struct __timespec64 tv;
- __clock_gettime64 (CLOCK_MONOTONIC, &tv);
- var ^= tv.tv_nsec;
+ __clock_gettime64 (CLOCK_REALTIME, &tv);
+ v = mix_random_values (v, tv.tv_sec);
+ v = mix_random_values (v, tv.tv_nsec);
#endif
- return 2862933555777941757 * var + 3037000493;
+
+ *r = mix_random_values (v, clock ());
+ return false;
}
#if _LIBC
@@ -213,7 +247,7 @@ static const char letters[] =
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
@@ -261,25 +295,17 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
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);
+ /* A random variable. */
+ random_value v = 0;
- /* How many random base-62 digits can currently be extracted from V. */
+ /* A value derived from the random variable, and how many random
+ base-62 digits can currently be extracted from VDIGBUF. */
+ random_value vdigbuf;
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
+ /* Least biased value for V. If V is less than this, V can generate
+ BASE_62_DIGITS unbiased digits. Otherwise the digits are biased. */
+ random_value const biased_min
= RANDOM_VALUE_MAX - RANDOM_VALUE_MAX % BASE_62_POWER;
len = strlen (tmpl);
@@ -299,18 +325,16 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
{
if (vdigits == 0)
{
- do
- {
- v = random_bits (v, use_getrandom);
- use_getrandom = true;
- }
- while (unfair_min <= v);
+ /* Worry about bias only if the bits are high quality. */
+ while (random_bits (&v, v) && biased_min <= v)
+ continue;
+ vdigbuf = v;
vdigits = BASE_62_DIGITS;
}
- XXXXXX[i] = letters[v % 62];
- v /= 62;
+ XXXXXX[i] = letters[vdigbuf % 62];
+ vdigbuf /= 62;
vdigits--;
}
--
2.37.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] Merge _GL_UNUSED C23 patch from Gnulib
2022-08-22 20:58 [PATCH 1/3] Merge _GL_UNUSED C23 patch from Gnulib Paul Eggert
2022-08-22 20:58 ` [PATCH 2/3] Merge getopt " Paul Eggert
2022-08-22 20:58 ` [PATCH 3/3] Merge tempname ASLR etc. " Paul Eggert
@ 2022-08-23 7:16 ` Florian Weimer
2 siblings, 0 replies; 7+ messages in thread
From: Florian Weimer @ 2022-08-23 7:16 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
* Paul Eggert:
> * posix/getopt.c (_getopt_initialize):
> * sysdeps/posix/tempname.c (try_dir, try_nocreate):
> Put _GL_UNUSED before args instead of after.
> This makes no difference for glibc.
> It is needed for Gnulib when being compiled on
> non-GCC C23 compilers.
This looks okay.
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] Merge tempname ASLR etc. patch from Gnulib
2022-08-22 20:58 ` [PATCH 3/3] Merge tempname ASLR etc. " Paul Eggert
@ 2022-08-23 7:19 ` Florian Weimer
2022-08-24 4:15 ` Paul Eggert
0 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2022-08-23 7:19 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
* Paul Eggert:
> @@ -299,18 +325,16 @@ try_tempname_len (char *tmpl, int suffixlen, void *args,
> {
> if (vdigits == 0)
> {
> - do
> - {
> - v = random_bits (v, use_getrandom);
> - use_getrandom = true;
> - }
> - while (unfair_min <= v);
> + /* Worry about bias only if the bits are high quality. */
> + while (random_bits (&v, v) && biased_min <= v)
> + continue;
Should glibc simply use arc4random_uniform here?
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] Merge getopt patch from Gnulib
2022-08-22 20:58 ` [PATCH 2/3] Merge getopt " Paul Eggert
@ 2022-08-23 7:19 ` Florian Weimer
0 siblings, 0 replies; 7+ messages in thread
From: Florian Weimer @ 2022-08-23 7:19 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
* Paul Eggert:
> * posix/getopt.c [!_LIBC]: Merge _WIN32 patch from Gnulib
> so that these source files are identical.
> This makes no difference for glibc.
> ---
> posix/getopt.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/posix/getopt.c b/posix/getopt.c
> index a160a4e3bd..128dc7fcf5 100644
> --- a/posix/getopt.c
> +++ b/posix/getopt.c
> @@ -45,7 +45,8 @@
> # define _(msgid) gettext (msgid)
> /* When used standalone, flockfile and funlockfile might not be
> available. */
> -# ifndef _POSIX_THREAD_SAFE_FUNCTIONS
> +# if (!defined _POSIX_THREAD_SAFE_FUNCTIONS \
> + || (defined _WIN32 && ! defined __CYGWIN__))
> # define flockfile(fp) /* nop */
> # define funlockfile(fp) /* nop */
> # endif
Okay.
Reviewed-by: Florian Weimer <fweimer@redhat.com>
Thanks,
Florian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] Merge tempname ASLR etc. patch from Gnulib
2022-08-23 7:19 ` Florian Weimer
@ 2022-08-24 4:15 ` Paul Eggert
0 siblings, 0 replies; 7+ messages in thread
From: Paul Eggert @ 2022-08-24 4:15 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 8/23/22 02:19, Florian Weimer wrote:
> Should glibc simply use arc4random_uniform here?
Unfortunately not, because tempname must use getrandom with
GRND_NONBLOCK and arc4random_uniform does not do that.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-24 4:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-22 20:58 [PATCH 1/3] Merge _GL_UNUSED C23 patch from Gnulib Paul Eggert
2022-08-22 20:58 ` [PATCH 2/3] Merge getopt " Paul Eggert
2022-08-23 7:19 ` Florian Weimer
2022-08-22 20:58 ` [PATCH 3/3] Merge tempname ASLR etc. " Paul Eggert
2022-08-23 7:19 ` Florian Weimer
2022-08-24 4:15 ` Paul Eggert
2022-08-23 7:16 ` [PATCH 1/3] Merge _GL_UNUSED C23 " Florian Weimer
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).