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?

Reply via email to