From patchwork Tue Oct 10 19:43:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 77439 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A591F3857709 for ; Tue, 10 Oct 2023 19:43:47 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by sourceware.org (Postfix) with ESMTPS id 102353858D28 for ; Tue, 10 Oct 2023 19:43:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 102353858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-pf1-x436.google.com with SMTP id d2e1a72fcca58-690bf8fdd1aso4483746b3a.2 for ; Tue, 10 Oct 2023 12:43:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1696967010; x=1697571810; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=wjnIqixaLorvWrSBDoHN2govR3lnolbxMeg1WlG+3LY=; b=LY3iHiYe5QsrdQbI8IlTgD30pGCwuSSNsn0PFvCfnIjH3Are1sklyxHT+VZW+34YoK rcfWIMabQHpSO9/Lubmv6yOAuWJmYhe9/ab1h8BrzFrrwi9Ar2B9qh18VOCDQdZcgDX0 jG0Up3ofmBbikyeH66svh+BpcEdMCUruUSk2C+wYmIhiD11gGCTmKS9+3JeODQpc2R89 q81NugROk+48Kvnr8ry5M0MqcbUYofbPKCjpMMBqoJ4VwTv3J9vvHxpy7dolfFHYt+lk xF6BaqE8TA7zSYBpmis4d59GYnk57QAsdP4UTcyAynOcG7Bx7w3ot/h2oggvrHZQjHC3 sLbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696967010; x=1697571810; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=wjnIqixaLorvWrSBDoHN2govR3lnolbxMeg1WlG+3LY=; b=Rgh6OgiyPZVxwrdhGF3Ox+xAMOW2joyT8JN9PJiXQlTP7D7mvj2rxjPnjFghpvJQ3p v7xbiZEETaRHBweE1vOg9MBKMzu4kWfr/ChKhHVUwzN+OqfJblRxnCnMCxJ4mQJgOI3v SNVec9q7hXHMOsgqRtkleXTIeXWYhJcdNPrvWrzXbkkPTb+azo/xL9dfP9s3z5gK7PqJ i36JJHO5E++I9XAsPRwoVbwDyvntUPbZAZrSrgqPr8Jtw6x/e1PCpJiA3G1cF7fczBlt g0sdJx1oxfp3P2yuGoPkB/7hxS9Eg0FG5Qt4nKoUaji/hl1cbdaXnfqFULBjaATqSYyz hpqA== X-Gm-Message-State: AOJu0Yy66l0QIc1scfv25KMnzc7mx5GucIhvcoCDyiZnu0IO36om5oqw MBiKp9W5xSSLkqARDZVEj5ToES09DAWOA33MVZDlyw== X-Google-Smtp-Source: AGHT+IFQQJeOmnqgoN8Dz6Mltqz/8mkO2bY082Fr8+aB9Q3/v8/gyczI2gtsZZhEY3aL2YQQ45f84A== X-Received: by 2002:a05:6a20:dda6:b0:135:1af6:9a01 with SMTP id kw38-20020a056a20dda600b001351af69a01mr14917282pzb.8.1696967010301; Tue, 10 Oct 2023 12:43:30 -0700 (PDT) Received: from mandiga.. ([2804:1b3:a7c2:d09b:ef2e:7c42:5ecf:a4ef]) by smtp.gmail.com with ESMTPSA id 12-20020a17090a01cc00b00276cb03a0e9sm11420062pjd.46.2023.10.10.12.43.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Oct 2023 12:43:29 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org, Paul Eggert Subject: [PATCH] posix: Sync tempname with gnulib (BZ 30958) Date: Tue, 10 Oct 2023 16:43:25 -0300 Message-Id: <20231010194325.2443369-1-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org The gnulib version contains an important fix (9ce573cde), which fixes some problems multithreading, entropy loss, and ASLR info. It also fixes a current issue where getrandom is not being used on some new files generation (only for __GT_NOCREATE on first try). The 044bf893ac removed __path_search, which is now moved to its own sysdeps/posix/pathsearch.c implementation. The bug report is not really againt glibc (the reported stated it checked on mingw); and Linux is unlikely to show the same issue even if ASLR is disabled (it might occur without ASLR and with a very imprecise clock). It syncs with gnulib commit 32a72f45374c9a36afa574d1a08bb98090270012. Checked on x86_64-linux-gnu. --- include/time.h | 2 + stdio-common/Makefile | 1 + stdio-common/pathsearch.c | 30 ++++++ stdio-common/tempname.c | 12 --- sysdeps/mach/hurd/clock.c | 1 + sysdeps/posix/clock.c | 1 + sysdeps/posix/pathsearch.c | 93 ++++++++++++++++++ sysdeps/posix/tempname.c | 165 ++++++++++---------------------- sysdeps/unix/sysv/linux/clock.c | 1 + time/clock.c | 1 + 10 files changed, 183 insertions(+), 124 deletions(-) create mode 100644 stdio-common/pathsearch.c create mode 100644 sysdeps/posix/pathsearch.c diff --git a/include/time.h b/include/time.h index f599eeed4e..94535eb1de 100644 --- a/include/time.h +++ b/include/time.h @@ -340,6 +340,8 @@ extern __time64_t __time64 (__time64_t *timer); libc_hidden_proto (__time64) #endif +libc_hidden_proto (clock); + /* Use in the clock_* functions. Size of the field representing the actual clock ID. */ #define CLOCK_IDFIELD_SIZE 3 diff --git a/stdio-common/Makefile b/stdio-common/Makefile index e88a9cea29..154b90f1a6 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -61,6 +61,7 @@ routines := \ itoa-digits \ itoa-udigits \ itowa-digits \ + pathsearch \ perror \ printf \ printf-prs \ diff --git a/stdio-common/pathsearch.c b/stdio-common/pathsearch.c new file mode 100644 index 0000000000..264b1cde57 --- /dev/null +++ b/stdio-common/pathsearch.c @@ -0,0 +1,30 @@ +/* Path search algorithm, for tmpnam, tmpfile, etc. Generic version. + Copyright (C) 2023 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 Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 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 + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +/* Perform the "SVID path search malarkey" on DIR and PFX. Write a + template suitable for use in __gen_tempname into TMPL, bounded + by TMPL_LEN. */ +int +__path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, + int try_tmpdir) +{ + __set_errno (ENOSYS); + return -1; +} +stub_warning (__path_search) + diff --git a/stdio-common/tempname.c b/stdio-common/tempname.c index 8c660b38c6..443714d071 100644 --- a/stdio-common/tempname.c +++ b/stdio-common/tempname.c @@ -20,18 +20,6 @@ #include #include -/* Perform the "SVID path search malarkey" on DIR and PFX. Write a - template suitable for use in __gen_tempname into TMPL, bounded - by TMPL_LEN. */ -int -__path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, - int try_tmpdir) -{ - __set_errno (ENOSYS); - return -1; -} -stub_warning (__path_search) - /* Generate a (hopefully) unique temporary filename in DIR (if applicable), using template TMPL. KIND determines what to do with that name. It may be one of: diff --git a/sysdeps/mach/hurd/clock.c b/sysdeps/mach/hurd/clock.c index 6f510dfebd..649ec3b1e8 100644 --- a/sysdeps/mach/hurd/clock.c +++ b/sysdeps/mach/hurd/clock.c @@ -51,3 +51,4 @@ clock (void) return total; } +libc_hidden_def (clock) diff --git a/sysdeps/posix/clock.c b/sysdeps/posix/clock.c index 6aa113f78e..92f90c0245 100644 --- a/sysdeps/posix/clock.c +++ b/sysdeps/posix/clock.c @@ -29,3 +29,4 @@ clock (void) return buf.tms_utime + buf.tms_stime; } +libc_hidden_def (clock) diff --git a/sysdeps/posix/pathsearch.c b/sysdeps/posix/pathsearch.c new file mode 100644 index 0000000000..18198e86fc --- /dev/null +++ b/sysdeps/posix/pathsearch.c @@ -0,0 +1,93 @@ +/* Path search algorithm, for tmpnam, tmpfile, etc. POSIX version. + Copyright (C) 2023 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 Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 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 + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +/* Return nonzero if DIR is an existent directory. */ +static bool +direxists (const char *dir) +{ + struct __stat64_t64 buf; + return __stat64_time64 (dir, &buf) == 0 && S_ISDIR (buf.st_mode); +} + +/* Path search algorithm, for tmpnam, tmpfile, etc. If DIR is + non-null and exists, uses it; otherwise uses the first of $TMPDIR, + P_tmpdir, /tmp that exists. Copies into TMPL a template suitable + for use with mk[s]temp. Will fail (-1) if DIR is non-null and + doesn't exist, none of the searched dirs exists, or there's not + enough space in TMPL. */ +int +__path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, + int try_tmpdir) +{ + const char *d; + size_t dlen, plen; + + if (!pfx || !pfx[0]) + { + pfx = "file"; + plen = 4; + } + else + { + plen = strlen (pfx); + if (plen > 5) + plen = 5; + } + + if (try_tmpdir) + { + d = __libc_secure_getenv ("TMPDIR"); + if (d != NULL && direxists (d)) + dir = d; + else if (dir != NULL && direxists (dir)) + /* nothing */ ; + else + dir = NULL; + } + if (dir == NULL) + { + if (direxists (P_tmpdir)) + dir = P_tmpdir; + else if (strcmp (P_tmpdir, "/tmp") != 0 && direxists ("/tmp")) + dir = "/tmp"; + else + { + __set_errno (ENOENT); + return -1; + } + } + + dlen = strlen (dir); + while (dlen > 1 && dir[dlen - 1] == '/') + dlen--; /* remove trailing slashes */ + + /* check we have room for "${dir}/${pfx}XXXXXX\0" */ + if (tmpl_len < dlen + 1 + plen + 6 + 1) + { + __set_errno (EINVAL); + return -1; + } + + sprintf (tmpl, "%.*s/%.*sXXXXXX", (int) dlen, dir, (int) plen, pfx); + return 0; +} diff --git a/sysdeps/posix/tempname.c b/sysdeps/posix/tempname.c index f24d962288..bf362db6e1 100644 --- a/sysdeps/posix/tempname.c +++ b/sysdeps/posix/tempname.c @@ -20,16 +20,9 @@ # include "tempname.h" #endif -#include -#include -#include - #include #include -#ifndef P_tmpdir -# define P_tmpdir "/tmp" -#endif #ifndef TMP_MAX # define TMP_MAX 238328 #endif @@ -43,12 +36,10 @@ # error report this to bug-gnulib@gnu.org #endif -#include #include #include #include -#include #include #include #include @@ -56,14 +47,12 @@ #if _LIBC # define struct_stat64 struct __stat64_t64 -# define __secure_getenv __libc_secure_getenv #else # define struct_stat64 struct stat # define __gen_tempname gen_tempname # define __mkdir mkdir # define __open open # define __lstat64_time64(file, buf) lstat (file, buf) -# define __stat64(file, buf) stat (file, buf) # define __getrandom getrandom # define __clock_gettime64 clock_gettime # define __timespec64 timespec @@ -77,94 +66,56 @@ 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) { - 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. */ - struct __timespec64 tv; - __clock_gettime64 (CLOCK_MONOTONIC, &tv); - var ^= tv.tv_nsec; -#endif - return 2862933555777941757 * var + 3037000493; + /* 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; } -#if _LIBC -/* Return nonzero if DIR is an existent directory. */ -static int -direxists (const char *dir) -{ - struct_stat64 buf; - return __stat64_time64 (dir, &buf) == 0 && S_ISDIR (buf.st_mode); -} +/* 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. -/* Path search algorithm, for tmpnam, tmpfile, etc. If DIR is - non-null and exists, uses it; otherwise uses the first of $TMPDIR, - P_tmpdir, /tmp that exists. Copies into TMPL a template suitable - for use with mk[s]temp. Will fail (-1) if DIR is non-null and - doesn't exist, none of the searched dirs exists, or there's not - enough space in TMPL. */ -int -__path_search (char *tmpl, size_t tmpl_len, const char *dir, const char *pfx, - int try_tmpdir) + 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) { - const char *d; - size_t dlen, plen; + /* Without GRND_NONBLOCK it can be blocked for minutes on some systems. */ + if (__getrandom (r, sizeof *r, GRND_NONBLOCK) == sizeof *r) + return true; - if (!pfx || !pfx[0]) - { - pfx = "file"; - plen = 4; - } - else - { - plen = strlen (pfx); - if (plen > 5) - plen = 5; - } + /* 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. - if (try_tmpdir) - { - d = __secure_getenv ("TMPDIR"); - if (d != NULL && direxists (d)) - dir = d; - else if (dir != NULL && direxists (dir)) - /* nothing */ ; - else - dir = NULL; - } - if (dir == NULL) - { - if (direxists (P_tmpdir)) - dir = P_tmpdir; - else if (strcmp (P_tmpdir, "/tmp") != 0 && direxists ("/tmp")) - dir = "/tmp"; - else - { - __set_errno (ENOENT); - return -1; - } - } + Of course we are in a state of sin here. */ - dlen = strlen (dir); - while (dlen > 1 && dir[dlen - 1] == '/') - dlen--; /* remove trailing slashes */ + random_value v = s; - /* check we have room for "${dir}/${pfx}XXXXXX\0" */ - if (tmpl_len < dlen + 1 + plen + 6 + 1) - { - __set_errno (EINVAL); - return -1; - } +#if _LIBC || (defined CLOCK_REALTIME && HAVE_CLOCK_GETTIME) + struct __timespec64 tv; + __clock_gettime64 (CLOCK_REALTIME, &tv); + v = mix_random_values (v, tv.tv_sec); + v = mix_random_values (v, tv.tv_nsec); +#endif - sprintf (tmpl, "%.*s/%.*sXXXXXX", (int) dlen, dir, (int) plen, pfx); - return 0; + *r = mix_random_values (v, clock ()); + return false; } -#endif /* _LIBC */ #if _LIBC static int try_tempname_len (char *, int, void *, int (*) (char *, void *), @@ -213,7 +164,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 +212,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 +242,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--; } diff --git a/sysdeps/unix/sysv/linux/clock.c b/sysdeps/unix/sysv/linux/clock.c index da005076e9..63c8baec35 100644 --- a/sysdeps/unix/sysv/linux/clock.c +++ b/sysdeps/unix/sysv/linux/clock.c @@ -34,3 +34,4 @@ clock (void) return (ts.tv_sec * CLOCKS_PER_SEC + ts.tv_nsec / (1000000000 / CLOCKS_PER_SEC)); } +libc_hidden_def (clock) diff --git a/time/clock.c b/time/clock.c index 3eb345b8c0..7c86701670 100644 --- a/time/clock.c +++ b/time/clock.c @@ -26,5 +26,6 @@ clock (void) __set_errno (ENOSYS); return (clock_t) -1; } +libc_hidden_def (clock) stub_warning (clock)