Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 24 Jan 2018 14:13:31 -0700
From:      Warner Losh <imp@bsdimp.com>
To:        "Conrad E. Meyer" <cem@freebsd.org>
Cc:        src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: 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/...
Message-ID:  <CANCZdfpQ7chPYYrs%2BO6EwPTeMJhWW=zYeeRnv5hvFda-LO24sQ@mail.gmail.com>
In-Reply-To: <CAG6CVpWTvE%2BFEpn2ScuoZG1je=c0xktnTO4iaO7ByZMLMU99xg@mail.gmail.com>
References:  <201801211542.w0LFgbsp005980@repo.freebsd.org> <CAG6CVpXxuFyHS11rF=NF6bSSkC2=xnDh=WnbK-aWp4sOomrZ7w@mail.gmail.com> <51ff8aef-5660-7857-e4d5-12cdc77bc071@FreeBSD.org> <20180124182548.X1063@besplex.bde.org> <CANCZdfpL7t_J6xXi8w%2BwtxE%2B4x8EA0AzqzQKno=dUKaJ_NXFrw@mail.gmail.com> <CAG6CVpWv32FjZggRCM3JNWCsMDOSXbVzQfP-dh73fei6Hqr5Mw@mail.gmail.com> <1516817048.42536.182.camel@freebsd.org> <CAG6CVpUpVLe5f=As44TOyePDTy-7LaTKA=rCkk-NjZvZkA5nQg@mail.gmail.com> <CANCZdfqZ5ZiMceh7NZ-YpbgsTW6gqwcxVun0XM_bY2%2BFpustKQ@mail.gmail.com> <2aa48cbd-247a-66cd-b486-02ee77ec2e96@selasky.org> <c7e000e9-6a21-be3e-4037-80e715224f15@FreeBSD.org> <CAG6CVpVo6=e_n7oFWzPeWjq5u%2BG%2BwSV6JvpKF-q4fmxxEJ_MUA@mail.gmail.com> <CANCZdfqxRcsE8MMvnvh-JzHwyb8QZiDpSB9uJmty%2BFQBJqiKcw@mail.gmail.com> <CAG6CVpWTvE%2BFEpn2ScuoZG1je=c0xktnTO4iaO7ByZMLMU99xg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jan 24, 2018 at 12:40 PM, Conrad Meyer <cem@freebsd.org> wrote:

> On Wed, Jan 24, 2018 at 11:27 AM, Warner Losh <imp@bsdimp.com> wrote:
> >
> > Which is why we should add check overflows for most of the no wait cases.
> > They should be checked, but not primarily with mallocarray...
>
> I don't understand what the distinction is here.  Can you help me
> understand why the overflow check should be lifted from mallocarray
> into the caller for no wait cases?  Or is that not what you're
> suggesting?
>

mallocarray should be the last line of defense, not the only line of
defense.

most of the time, it's more correct to say

if (WOULD_OVERFLOW(a,b))
    return EINVAL;
ptr = mallocarray(a,b...);

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

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

And that's why I have such a problem with mallocarray: it's only useful
when people are lazy and haven't checked, and then it creates a DoS path
for things that don't check. We'll change it now and think we're safe, when
we still have issues, just different issues than before. It may be a
necessary change, but it certainly isn't sufficient.

Warner



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfpQ7chPYYrs%2BO6EwPTeMJhWW=zYeeRnv5hvFda-LO24sQ>