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 20:36:45 +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>,
        Mark Harris <mark.hsj@gmail.com>,
        Eric Biggers <ebiggers@kernel.org>,
        linux-crypto@vger.kernel.org
Subject: Re: [PATCH v4] arc4random: simplify design for better safety
Message-ID: <YuA0PcpBdWU5MS9s@zx2c4.com>
References: <20220725225728.824128-1-Jason@zx2c4.com>
 <20220726133049.1145913-1-Jason@zx2c4.com>
 <45ef8ca0-12ca-4853-98a0-9f52dfca8c57@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <45ef8ca0-12ca-4853-98a0-9f52dfca8c57@linaro.org>
Precedence: bulk

Hi Adhemerval,

On Tue, Jul 26, 2022 at 01:20:11PM -0300, Adhemerval Zanella Netto wrote:
> > +	{
> > +	  atomic_store_relaxed (&have_getrandom, false);
> 
> I still think there is no much gain in this optimization, the syscall will
> most likely be present and it is one less static data.  Also, we avoid to
> use __ASSUME_GETRANDOM on generic code (all __ASSUME usage within
> sysdeps and/or nptl).

Oh! *That's* what you were talking about before. Sorry I didn't catch
your meaning the first time through.

Okay so you're alright having +1 syscall overhead on old systems, so
that new systems can have a byte less of static data. I don't hold any
opinions either way there and will defer to your expertise, so I'll get
rid of this part on v5.

> > +    __ppoll_infinity_nocancel;
> >      __sigtimedwait;
> >      # functions used by nscd
> >      __netlink_assert_response;
> 
> There is no need to export on GLIBC_PRIVATE, since it is not currently usage
> libc.so.  Just define is a hidden (attribute_hidden).
> Use attribute_hidden here and remove it from sysdeps/unix/sysv/linux/Versions.
>> Maybe just add an inline wrapper on sysdeps/unix/sysv/linux/not-cancel.h, 
> as for __getrandom_nocancel:
> It avoids a lot of boilerplate code to add the internal symbol.

Okay I'll skip all the symbol stuff and just do the static inline like
getrandom has. Thanks for the suggestion; that's a lot simpler.

> Also update the hurd sysdeps/mach/hurd/not-cancel.h with a wrapper to 
> __poll (since it does not really support pthread cancellation).

Ack.

Thanks for the comments. v5 coming up shortly.

Jason