Date: Wed, 24 Jan 2018 13:40:35 -0800 From: Conrad Meyer <cem@freebsd.org> To: Warner Losh <imp@bsdimp.com> 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: <CAG6CVpXbV9uNNom%2Bc5zzD=ykXM8i_Uyj4krekoyNeHUkC49ekQ@mail.gmail.com> In-Reply-To: <CANCZdfpQ7chPYYrs%2BO6EwPTeMJhWW=zYeeRnv5hvFda-LO24sQ@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> <CANCZdfpQ7chPYYrs%2BO6EwPTeMJhWW=zYeeRnv5hvFda-LO24sQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Jan 24, 2018 at 1:13 PM, Warner Losh <imp@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpXbV9uNNom%2Bc5zzD=ykXM8i_Uyj4krekoyNeHUkC49ekQ>