Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 26 Jul 2022 14:20:38 +0200
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Cc:     libc-alpha@sourceware.org, Florian Weimer <fweimer@redhat.com>,
        Cristian =?utf-8?Q?Rodr=C3=ADguez?= <crrodriguez@opensuse.org>,
        Paul Eggert <eggert@cs.ucla.edu>, linux-crypto@vger.kernel.org
Subject: Re: [PATCH v2] arc4random: simplify design for better safety
Message-ID: <Yt/cFoln0FvK7hn7@zx2c4.com>
References: <20220725225728.824128-1-Jason@zx2c4.com>
 <20220725232810.843433-1-Jason@zx2c4.com>
 <9c576e6b-77c9-88c5-50a3-a43665ea5e93@linaro.org>
 <Yt/V78eyHIG/kms3@zx2c4.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Yt/V78eyHIG/kms3@zx2c4.com>
Precedence: bulk

On Tue, Jul 26, 2022 at 01:54:23PM +0200, Jason A. Donenfeld wrote:
> > Also, does it make sense to fallback if we build for a kernel that should
> > always support getrandom?
> 
> I guess only if syscall filtering is a concern. But if not, then maybe
> yea? We could do this in a follow-up commit, or I could do this in v4.
> Would `#if __LINUX_KERNEL_VERSION >` be the right mechanism to use here?
> If so, I think the way I'd implement that would be:
>
> [...]
>
> And then arc4random_getrandom_failure() being a noreturn function would
> make gcc optimize out the rest.
> 
> Does that seem like a good approach?

It actually winds up looking a bit more like the below. Let me know if
you want that in v4.

diff --git a/stdlib/arc4random.c b/stdlib/arc4random.c
index c0f132ea9b..8fcf41e7de 100644
--- a/stdlib/arc4random.c
+++ b/stdlib/arc4random.c
@@ -43,7 +43,7 @@ __arc4random_buf (void *p, size_t n)
     {
       ssize_t l;

-      if (!atomic_load_relaxed (&have_getrandom))
+      if (!__ASSUME_GETRANDOM && !atomic_load_relaxed (&have_getrandom))
 	break;

       l = __getrandom_nocancel (p, n, 0);
@@ -59,7 +59,7 @@ __arc4random_buf (void *p, size_t n)
 	arc4random_getrandom_failure (); /* Weird, should never happen.  */
       else if (l == -EINTR)
 	continue; /* Interrupted by a signal; keep going.  */
-      else if (l == -ENOSYS)
+      else if (!__ASSUME_GETRANDOM && l == -ENOSYS)
 	{
 	  atomic_store_relaxed (&have_getrandom, false);
 	  break; /* No syscall, so fallback to /dev/urandom.  */
diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
index 74adc3956b..75d5f953d4 100644
--- a/sysdeps/unix/sysv/linux/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/kernel-features.h
@@ -236,4 +236,11 @@
 # define __ASSUME_FUTEX_LOCK_PI2 0
 #endif

+/* The getrandom() syscall was added in 3.17.  */
+#if __LINUX_KERNEL_VERSION >= 0x031100
+# define __ASSUME_GETRANDOM 1
+#else
+# define __ASSUME_GETRANDOM 0
+#endif
+
 #endif /* kernel-features.h */