So, I don't understand this changes and the followup change by kre.
Why, exactly, do we need "a more secure random generator" to randomly drop
packets from a queue? What was the actual complaint by coverity and why
wasn't it a false positive?
The followup commit by kre partiallly undid the change for "rump and
anything else userland". Why is this necessary for rump?
AFAICT the kernel rng subsystem works just fine in rump servers, so
presumably cprng_fast* is available there. Hence it should be possible to
fix this correctly instead of pampering over the real issue.[1]
At the very least I would have expected a comment in the code why using
cprng_fast32 in rump kernels is not possible. And what specifically the
"anything else userland" is.
--chris
[1] It seems to me that pampering over over rump failures instead of
properly fixing the issues as the crop up has become kind of a sport in
recent years.
On Sat, Jan 18, 2025 at 04:51:28PM +0000, Emmanuel wrote:
> Module Name: src
> Committed By: joe
> Date: Sat Jan 18 16:51:28 UTC 2025
>
> Modified Files:
> src/sys/altq: altq_classq.h
>
> Log Message:
> coverity scan: use a more secure random generator when randomly selecting a
> packet in queue
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.9 -r1.10 src/sys/altq/altq_classq.h
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
> Modified files:
>
> Index: src/sys/altq/altq_classq.h
> diff -u src/sys/altq/altq_classq.h:1.9 src/sys/altq/altq_classq.h:1.10
> --- src/sys/altq/altq_classq.h:1.9 Wed Jan 8 13:00:04 2025
> +++ src/sys/altq/altq_classq.h Sat Jan 18 16:51:28 2025
> @@ -1,4 +1,4 @@
> -/* $NetBSD: altq_classq.h,v 1.9 2025/01/08 13:00:04 joe Exp $ */
> +/* $NetBSD: altq_classq.h,v 1.10 2025/01/18 16:51:28 joe Exp $ */
> /* $KAME: altq_classq.h,v 1.6 2003/01/07 07:33:38 kjc Exp $ */
>
> /*
> @@ -39,6 +39,8 @@
> #ifndef _ALTQ_ALTQ_CLASSQ_H_
> #define _ALTQ_ALTQ_CLASSQ_H_
>
> +#include <sys/cprng.h>
> +
> #ifdef __cplusplus
> extern "C" {
> #endif
> @@ -155,7 +157,7 @@ _getq_random(class_queue_t *q)
> else {
> struct mbuf *prev = NULL;
>
> - n = random() % qlen(q) + 1;
> + n = cprng_fast32() % qlen(q) + 1;
> for (i = 0; i < n; i++) {
> prev = m;
> m = m->m_nextpkt;
>
