Wider adoption of mallocarray(9).

[pfg]

Summary

I made a sweep through the tree looking for kernel mallocs with a
multiplication and M_NOWAIT.

Test Plan

Buildworld passed, Universe still building.

[cem]

Was this mechanical (with e.g. coccinelle)?

No reason to limit this to M_NOWAIT, I think.

[delphij]

Is it possible that you can split this into smaller pieces (for instance, archictecture code, CAM/CTL, ABI, drivers that ideally be splitted into smaller ones, etc)? Reviewing a changeset this big can quickly get distracted.

The arc4random change looks good to me, by the way, perhaps you can use a separate changeset so O3: Kernel Random Number Generator will not block everyone else?

sys/dev/mxge/if_mxge.c

3322 ↗

(On Diff #37750)

Should these be changed too?

3325 ↗

(On Diff #37750)

ditto.

3329 ↗

(On Diff #37750)

ditto

3332 ↗

(On Diff #37750)

ditto.

[pfg]

In D13837#290278, @cem wrote:

Was this mechanical (with e.g. coccinelle)?

Nope ... I checked it on opengrok and then by hand.

No reason to limit this to M_NOWAIT, I think.

It is now the case that it panics right (OpenBSD panics also, I think)?
Can we have it return NULL for M_NOWAIT and panic for M_WAITOK?

[cem]

In D13837#290288, @pfg wrote:

It is now the case that it panics right

Right.

(OpenBSD panics also, I think)?

No idea.

Can we have it return NULL for M_NOWAIT and panic for M_WAITOK?

No, I don't think that makes any sense.

[pfg]

When I started this changes, mallocarray(9) would return NULL on overflow, and for the M_WAITOK case the code was not prepared to handle that, so I avoided M_WAITOK changes for this revision (I was not going to do it at all).

If we now panic then there is a case for using it for M_WAITOK as well, but I will take a break until this settles.

sys/dev/mxge/if_mxge.c

3322 ↗

(On Diff #37750)

M_WAITOK requires another sweep over the tree. I prefer not to mix them now.

[pfg]

In D13837#290289, @cem wrote:

In D13837#290288, @pfg wrote:

It is now the case that it panics right

Right.

(OpenBSD panics also, I think)?

No idea.

Can we have it return NULL for M_NOWAIT and panic for M_WAITOK?

No, I don't think that makes any sense.

For the M_NOWAIT case this matches the calloc(3) and reallocarray(3) behaviour and we can handle that gracefully. For M_WAITOK, NULL is not an option so it's sane to panic.

[cem]

In D13837#290292, @pfg wrote:

For the M_NOWAIT case this matches the calloc(3) and reallocarray(3) behaviour

I think calloc(3) and reallocarray(3) are in the wrong. It should probably assert(), abort(), or exit() instead.

and we can handle that gracefully. For M_WAITOK, NULL is not an option so it's sane to panic.

It's insane not to panic when an overflowing allocation was requested. Flags do not change that an overflow was requested.

[pfg]

Drop the arc4random change for now as to unblock this change.

Some parts of the bigger patch (that have a maintainer) have been
committed. I considered breaking a part the differential per area but that
takes some time and I will be focusing on other things.

[shurd]

Looks good to me as far as bnxt and iflib are concerned.

[pfg]

I committed sys/dev in rr327949 so drop those from the Revision.

[pfg]

For the record: the changes were committed by parts in a series of commits from r328016 to r328026.