svn commit: r345380 - head/lib/libc/gen

[Bruce Evans]

On Thu, 21 Mar 2019, Conrad Meyer wrote:

> Log:
>  arc4random: Adjust example code to use uniform() API

This changes the example from correct to broken.

I see 4 bugs:

1. The log message says that the example uses uniform(), but it actually
    uses arc4random_uniform().

>  PR:		236678
>  Reported by:	Andras Farkas <deepbluemistake AT gmail.com>
>
> Modified:
>  head/lib/libc/gen/arc4random.3
>
> Modified: head/lib/libc/gen/arc4random.3
> ==============================================================================
> --- head/lib/libc/gen/arc4random.3	Thu Mar 21 14:17:55 2019	(r345379)
> +++ head/lib/libc/gen/arc4random.3	Thu Mar 21 14:45:08 2019	(r345380)
> @@ -31,7 +31,7 @@
> .\" Manual page, using -mandoc macros
> .\" $FreeBSD$
> .\"
> -.Dd July 19, 2014
> +.Dd March 21, 2019
> .Dt ARC4RANDOM 3
> .Os
> .Sh NAME
> @@ -116,7 +116,7 @@ and
> functions using
> .Fn arc4random :

2. This description still says that the example uses arc4random(), but it
actually uses arc4random_uniform().

> .Pp
> -.Dl "#define foo4random() (arc4random() % ((unsigned)RAND_MAX + 1))"
> +.Dl "#define foo4random() (arc4random_uniform(RAND_MAX + 1))"
> .Sh SEE ALSO
> .Xr rand 3 ,
> .Xr rand48 3 ,

3. RAND_MAX is usually INT_MAX, so adding 1 to it gives undefined behavior.
    The previous version carefully converted RAND_MAX to unsigned to get
    defined behaviour.

The PR doesn't have this bug.  It gives the different bad example of using
(unsigned)RAND_MAX.  This doesn't overflow, but is bad in other ways.  Its
cast to unsigned is bogus without adding 1.  Without adding 1, the range is
unusual and the example is different.  When misemulating rand() using
random() it is a common bug to take the result modulo RAND_MAX.  This gives
the very non-uniform distribution of never returning RAND_MAX.

rand() is not specified to have a uniform distribution, but quality of
implementation requires this.  This was broken in 4.4BSD but is fixed in
FreeBSD.  The fix involves reducing RAND_MAX by 2 to get a uniform
distribution in the supported range.  The old version was obviously not
uniform since it took values mod 0x7fffffff instead of mod 0x7fffffffU + 1,
so it missed 1 value in the full 31-bit range.  The it adjusted an
intermediate value of 0, which makes it unclear what the final range is.

Oops.  Since RAND_MAX is now 2 smaller than INT_MAX, adding 1 to it doesn't
cause overflow.  So the badness in the current example is just a style bug.

4. Another (old) bug in the example is that it says that the #define for
    foo4random() produces a drop-in replacement for [both] rand() and random().
    The maximum value for random() is still 0x7fffffff, so the same #define
    doesn't work for both, and it only ever worked accidentally if
    RAND_MAX happens to be 0x7fffffff, but RAND_MAX has been 0x7ffffffd since
    before this example existed.

Bruce