svn commit: r328218 - in head/sys: amd64/amd64 arm/xscale/ixp425 arm64/arm64 cam cam/ctl compat/ndis dev/aacraid dev/advansys dev/ath dev/beri/virtio dev/bnxt dev/bwn dev/ciss dev/cxgbe/crypto dev/...

[Conrad Meyer]

On Wed, Jan 24, 2018 at 1:13 PM, Warner Losh <imp at bsdimp.com> wrote:
> mallocarray should be the last line of defense, not the only line of
> defense.

Agreed.

> most of the time, it's more correct to say
>
> if (WOULD_OVERFLOW(a,b))
>     return EINVAL;
> ptr = mallocarray(a,b...);

Disagree -- the right check to have outside is much more constrained
than just "will this overflow?"  A 10GB allocation request is still
insane on amd64, just for a different reason than on i386.  I think
BDE said something along the lines of max 32 kB for most allocations,
and I don't really disagree with that.  Picking a specific number for
mallocarray itself (other than overflow) restricts the generality,
though.

> since an error return, assuming it's handled correctly is preferable to a
> panic.

Agreed.

> I thought this was more true for NOWAIT than for WAITOK cases, but I've
> realized it's more true always.

Yeah, I think it's equally true of M_WAITOK and M_NOWAIT.

> And that's why I have such a problem with mallocarray: it's only useful when
> people are lazy and haven't checked,

It's useful as a seatbelt.  Empirically, people are not perfect at
doing the checking.  I can understand that it feels like admitting
laziness to accept that we need a final seatbelt check at all, but I
don't think having the seatbelt hurts.

> and then it creates a DoS path for
> things that don't check.

No, again; it doesn't "create" a DoS.  Any DoS path was already
present as a heap corruption path.  The DoS is strictly an
improvement.

> We'll change it now and think we're safe, when we
> still have issues, just different issues than before.

Don't think that, then?  We have replaced some classes of heap
corruption with a DoS.  That's it; nothing more.  I don't think anyone
promoting mallocarray is overrepresenting what it does or claims to
do.

> It may be a necessary
> change, but it certainly isn't sufficient.

I don't disagree.

Best,
Conrad