On 29.07.2017 16:19, Taylor R Campbell wrote:
> It's stupid that we have to litter drivers with
>
> if (SIZE_MAX/sizeof(struct xyz_cookie) < iocmd->ncookies) {
> error = EINVAL;
> goto out;
> }
> cookies = kmem_alloc(iocmd->ncookies*sizeof(struct xyz_cookie),
> KM_SLEEP);
> ...
>
> and as you can tell from some recent commits, it hasn't been done
> everywhere. It's been a consistent source of problems in the past.
>
> This multiplication overflow check, which is all that most drivers do,
> also doesn't limit the amount of wired kmem that userland can request,
> and there's no way for kmem to say `sorry, I can't satisfy this
> request: it's too large' other than to panic or wedge.
>
> In userland we now have reallocarr(3). I propose that we add
> something to the kernel, but I'm not immediately sure what it should
> look like because kernel is a little different. Solaris/illumos
> doesn't seem to have anything we could obviously parrot, from a
> cursory examination.
>
> We could add something like
>
> void *kmem_allocarray(size_t n, size_t size, int flags);
> void kmem_freearray(size_t n, size_t size);
>
> That wouldn't address bounding the amount of wired kmem userland can
> request. Perhaps that's OK: perhaps it's enough to have drivers put
> limits on the number of (say) array elements at the call site,
> although then there's not as much advantage to having a new API.
> Instead, we could make it
>
> void *kmem_allocarray(size_t n, size_t size, size_t maxn,
> int flags);
> or
> void *kmem_allocarray(size_t n, size_t size, size_t maxbytes,
> int flags);
>
> It's not clear that the call site is exactly the right place to
> compute a bound on the number of bytes a user can allocate. On the
> other hand, if it's not clear up front what the bound is, then that
> makes a foot-oriented panic gun, or an instawedge, if the kernel can
> decides at run-time how many bytes is more than it can possibly ever
> satisfy, which is not so great either. If you specify up front in the
> source, at least you can say by examination of the source whether it
> has a chance of working or not on some particular platform. And maybe
> we can make it easier to write an expression for `no more than 10% of
> the machine's current RAM' or something.
>
> Either way, kmem_allocarray would have to have the option of returning
> NULL, unlike kmem_alloc(..., KM_SLEEP), which is a nontrivial change
> to the contract now that chuq@ recently dove in deep to make sure it
> never returns NULL.
>
> Thoughts?
>
I think we should go for kmem_reallocarr(). It has been designed for
overflows like realocarray(3) with an option to be capable to resize a
table fron 1 to N elements and back from N to 0 including freeing.
Attachment:
signature.asc
Description: OpenPGP digital signature