Re: Proposal: kmem_valloc [was: Re: raspberry pi panic 7.0_BETA after install fs resize]

[Maxime Villard <max%M00nBSD.net@localhost>]

Le 08/11/2014 18:06, Jean-Yves Migeon a écrit :
> Le 08/11/2014 07:28, Maxime Villard a écrit :
>  >[snip]
> > That's mostly what kern_malloc() does, but it is consistent and
> > sometimes consumes less memory - kern_malloc may allocate one more
> > page (PAGE_SIZE).
> >
> > As some of you may have noticed, some recent Security Advisories
> > were related to kmem. And now there's this issue in ffs_unmount();
> > and more bugs will come.
> >
> > Here is a patch which implements kmem_valloc.
> >
> > Comments?
>
> I agree that the size not being tracked by the allocator (well, it is,
> but the API is ill-designed in this regard) leads to great bugs.
>
> Two things come to mind:
>
> - I think that KMEM_SIZE should become enabled by default instead (and
> not reserved to DIAGNOSTIC kernels).

Then what's the point of giving a size, if the kernel already knows
what this size is?

> It feels weird to have the size
> field added twice when the option is enabled;

Yes it does...

> - I still believe that allocating 0 byte of memory should end in
> panic(). While standards make the result implementation-defined, to me
> it indicates that something went wrong.

I would agree with you, but the fact is that kmem_alloc(0) is
necessarily triggered when a variable is passed - I mean it is not hard-
coded -, and when memory is allocated depending on a variable, it is
generally filled in and used depending on this variable.

If 0 bytes are allocated, 0 bytes will be written/read. If it's not the
case, then the caller is buggy, and KMEM_REDZONE will detect an
overflow, if any.

Thus kmem_valloc(0) could return a valid address that must not be
touched.

> Having an empty memory region
> serves no purpose in kernel and will put pressure on the "kmem-8" cache
> for no benefit.

Yes; but remember that kmem_alloc(0) is rare - well, it should be rare -
so there won't be such a huge "pressure" on kmem-8.

> Returning NULL would be even worse from a security standpoint and poses
> problems for documentation. What does it mean to kmem_valloc(0,
> KM_SLEEP)? A successful allocation but with an invalid pointer? Huh.

A successful allocation with a valid pointer, but a pointer that must
not be written to nor read from.

> Cheers,