This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
Re: [PATCH 5/5] Add arc4random() etc. from OpenBSD 5.8
- From: Sebastian Huber <sebastian dot huber at embedded-brains dot de>
- To: newlib at sourceware dot org
- Date: Fri, 18 Mar 2016 19:07:09 +0100 (CET)
- Subject: Re: [PATCH 5/5] Add arc4random() etc. from OpenBSD 5.8
- Authentication-results: sourceware.org; auth=none
- References: <1458298168-20216-1-git-send-email-sebastian dot huber at embedded-brains dot de> <1458298168-20216-6-git-send-email-sebastian dot huber at embedded-brains dot de> <20160318112828 dot GK16696 at calimero dot vinschen dot de> <20160318135001 dot GM16696 at calimero dot vinschen dot de> <56EC1522 dot 4070406 at embedded-brains dot de> <20160318162502 dot GA27115 at calimero dot vinschen dot de> <20160318170141 dot GE27115 at calimero dot vinschen dot de>
----- Am 18. Mrz 2016 um 18:01 schrieb Corinna Vinschen vinschen@redhat.com:
> On Mar 18 17:25, Corinna Vinschen wrote:
>> On Mar 18 15:48, Sebastian Huber wrote:
>> > On 18/03/16 14:50, Corinna Vinschen wrote:
>> > >On Mar 18 12:28, Corinna Vinschen wrote:
>> > >>>On Mar 18 11:49, Sebastian Huber wrote:
>> > >>>> >According to the OpenBSD man page, "A Replacement Call for Random". It
>> > >>>> >offers high quality random numbers derived from input data obtained by
>> > >>>> >the OpenBSD specific getentropy() system call which is declared in
>> > >>>> ><unistd.h> and must be implemented for each Newlib port externally. The
>> > >>>> >arc4random() functions are used for example in LibreSSL and OpenSSH.
>> > >>>> >
>> > >>>> >Cygwin provides currently its own implementation of the arc4random
>> > >>>> >family. Maybe it makes sense to use this getentropy() implementation:
>> > >>>> >
>> > >>>> >http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/lib/libcrypto/crypto/getentropy_win.c?rev=1.4&content-type=text/x-cvsweb-markup
>> > >>>
>> > >>>No, that pulls in a dependency to another DLL which we're trying to avoid.
>> > >>>Using RtlGenRandom should work though. I have to test this.
>> > >All patches applied. I immediately implemented the changes required for
>> > >Cygwin. In the first place that required to add two functions
>> > >arc4random_stir and arc4random_addrandom to maintain backward
>> > >compatibility since both OpenBSD functions were already exported by
>> > >Cygwin.
>> >
>> > My aim with the "newlib/libc/sys/*/include/machine/_arc4random.h" file was
>> > to avoid #ifdef SYS_XYZ cascades in the generic files. It seems that for
>> > Cygwin there is no "newlib/libc/sys/cygwin" directory. Would it be possible
>> > to add this and place a
>> > "newlib/libc/sys/cygwin/include/machine/_arc4random.h" in it?
>>
>> That might be possible, but the arc4random.h file does not even have
>> provisions for redefining _ARC4_LOCK/_ARC4_UNLOCK. The file defines
>> them unconditionally.
>
> Patch proposal:
>
> commit dc12772137ad6fd65f8628e4f8b0625163e46e28
> Author: Corinna Vinschen <corinna@vinschen.de>
> Date: Fri Mar 18 18:01:07 2016 +0100
>
> Allow machine-dependent arc4 locking
>
> newlib:
> * libc/stdlib/arc4random.h: Remove Cygwin-specific locking code.
> Conditionalize arc4 locking. Check for _ARC4_LOCK_INIT being
> undefined to fall back to default implementation.
>
> cygwin:
> * include/machine/_arc4random.h: New file.
>
> Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
>
> diff --git a/newlib/libc/stdlib/arc4random.h b/newlib/libc/stdlib/arc4random.h
> index 8bb72f4..54bcbe8 100644
> --- a/newlib/libc/stdlib/arc4random.h
> +++ b/newlib/libc/stdlib/arc4random.h
> @@ -37,30 +37,17 @@
> #include <sys/lock.h>
> #include <signal.h>
>
> -__LOCK_INIT(static, _arc4random_mutex);
> +#ifndef _ARC4_LOCK_INIT
>
> -#ifdef __CYGWIN__
> -
> -extern int __isthreaded;
> -
> -#define _ARC4_LOCK() \
> - do { \
> - if (__isthreaded) \
> - __lock_acquire (_arc4random_mutex); \
> - } while (0)
> -
> -#define _ARC4_UNLOCK() \
> - do { \
> - if (__isthreaded) \
> - __lock_release (_arc4random_mutex); \
> - } while (0)
> -#else
> +#define _ARC4_LOCK_INIT __LOCK_INIT(static, _arc4random_mutex);
>
> #define _ARC4_LOCK() __lock_acquire(_arc4random_mutex)
>
> #define _ARC4_UNLOCK() __lock_release(_arc4random_mutex)
>
> -#endif
> +#endif /* _ARC4_LOCK_INIT */
> +
> +_ARC4_LOCK_INIT
>
> #ifdef _ARC4RANDOM_DATA
> _ARC4RANDOM_DATA
> diff --git a/winsup/cygwin/include/machine/_arc4random.h
> b/winsup/cygwin/include/machine/_arc4random.h
> new file mode 100644
> index 0000000..3ce2458
> --- /dev/null
> +++ b/winsup/cygwin/include/machine/_arc4random.h
> @@ -0,0 +1,30 @@
> +/* machine/_arc4random.h
> +
> + Copyright 2016 Red Hat, Inc.
> +
> +This file is part of Cygwin.
> +
> +This software is a copyrighted work licensed under the terms of the
> +Cygwin license. Please consult the file "CYGWIN_LICENSE" for
> +details. */
> +
> +#ifndef _MACHINE_ARC4RANDOM_H
> +#define _MACHINE_ARC4RANDOM_H
> +
> +extern int __isthreaded;
> +
> +#define _ARC4_LOCK_INIT __LOCK_INIT(static, _arc4random_mutex);
> +
> +#define _ARC4_LOCK() \
> + do { \
> + if (__isthreaded) \
> + __lock_acquire (_arc4random_mutex); \
> + } while (0)
> +
> +#define _ARC4_UNLOCK() \
> + do { \
> + if (__isthreaded) \
> + __lock_release (_arc4random_mutex); \
> + } while (0)
> +
> +#endif /* _MACHINE_ARC4RANDOM_H */
Out of curiosity, why is this __isthreaded stuff not covered by <sys/lock.h> or only necessary here, e.g. in contrast to for example the atexit lock?