On 2018/04/17 13:34, Renaud Allard wrote:
>
>
> On 04/17/2018 01:26 PM, Stuart Henderson wrote:
>
> > > I'm not going to object strongly, but this occurs twice:
> > >
> > > for (rnd = arc4random() % weights, i = 0; i < num_servers; i++)
> > >
> > > The expression
> > > arc4random() % weights;
> > >
> > > is subject to modulus bias. Use
> > >
> > > arc4random_uniform(weights);
> > >
> > > instead. I'm not sure this fixes all the problems with randomness in
> > > its vicinity, but it should be a bit better...
> >
> > This is something where a change to arc4random_uniform would make an actual
> > improvement so this one seems fair enough..
> >
>
> OK, does that one look like better?
> Index: mail/exim//patches/patch-src_spam_c
> ===================================================================
> RCS file: mail/exim//patches/patch-src_spam_c
> diff -N mail/exim//patches/patch-src_spam_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ mail/exim//patches/patch-src_spam_c 17 Apr 2018 11:30:31 -0000
> @@ -0,0 +1,33 @@
> +--- src/spam.c.orig Tue Apr 17 10:56:03 2018
> ++++ src/spam.c Tue Apr 17 13:26:42 2018
> +@@ -139,21 +139,11 @@
> + spamd_address_container * sd;
> + long rnd, weights;
> + unsigned pri;
> +-static BOOL srandomed = FALSE;
> +
> + /* speedup, if we have only 1 server */
> + if (num_servers == 1)
> + return (spamds[0]->is_failed ? -1 : 0);
> +
> +-/* init ranmod */
> +-if (!srandomed)
Honestly I would leave the srandom bits in. No major objection, but they
don't hurt and it will save you maintenance hassle merging patches when
they fix the typo in the comment you are removing in this patch!
Minimal patches are usually better.
> +- {
> +- struct timeval tv;
> +- gettimeofday(&tv, NULL);
> +- srandom((unsigned int)(tv.tv_usec/1000));
> +- srandomed = TRUE;
> +- }
> +-
> + /* scan for highest pri */
> + for (pri = 0, i = 0; i < num_servers; i++)
> + {
> +@@ -170,7 +160,7 @@
> + if (weights == 0) /* all servers failed */
> + return -1;
> +
> +-for (rnd = random() % weights, i = 0; i < num_servers; i++)
> ++for (rnd = arc4random_uniform(weights), i = 0; i < num_servers; i++)
> + {
> + sd = spamds[i];
> + if (!sd->is_failed && sd->priority == pri)
> Index: mail/exim//patches/patch-src_transports_smtp_socks_c
> ===================================================================
> RCS file: mail/exim//patches/patch-src_transports_smtp_socks_c
> diff -N mail/exim//patches/patch-src_transports_smtp_socks_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ mail/exim//patches/patch-src_transports_smtp_socks_c 17 Apr 2018
> 11:30:31 -0000
> @@ -0,0 +1,32 @@
> +--- src/transports/smtp_socks.c.orig Tue Apr 17 10:50:46 2018
> ++++ src/transports/smtp_socks.c Tue Apr 17 13:28:06 2018
> +@@ -161,20 +161,10 @@
> + socks_opts * lim = &proxies[nproxies];
> + long rnd, weights;
> + unsigned pri;
> +-static BOOL srandomed = FALSE;
> +
> + if (nproxies == 1) /* shortcut, if we have only 1 server */
> + return (proxies[0].is_failed ? -1 : 0);
> +
> +-/* init random */
> +-if (!srandomed)
> +- {
> +- struct timeval tv;
> +- gettimeofday(&tv, NULL);
> +- srandom((unsigned int)(tv.tv_usec/1000));
> +- srandomed = TRUE;
> +- }
> +-
> + /* scan for highest pri */
> + for (pri = 0, sd = proxies; sd < lim; sd++)
> + if (!sd->is_failed && sd->priority > pri)
> +@@ -187,7 +177,7 @@
> + if (weights == 0) /* all servers failed */
> + return -1;
> +
> +-for (rnd = random() % weights, i = 0; i < nproxies; i++)
> ++for (rnd = arc4random(weights), i = 0; i < nproxies; i++)
> + {
> + sd = &proxies[i];
> + if (!sd->is_failed && sd->priority == pri)
