Re: [KernelAllocator] Usage of GlobalAlloc

[Gary Guo <gary@garyguo.net>]

On Sat, 22 May 2021 06:46:41 -0400
Hanqing Zhao <ouckarlzhao@gmail.com> wrote:

> On Sat, May 22, 2021 at 4:22 AM Gary Guo <gary@garyguo.net> wrote:
> >
> > On Fri, 21 May 2021 13:42:40 -0400
> > Hanqing Zhao <ouckarlzhao@gmail.com> wrote:
> >  
> > > While implementing an alloc crate, I intend to pass custom
> > > parameters and extra parameters such as
> > > memory flags (GFP_KERNEL, GFP_ATOMIC, etc) through the `Layout`
> > > parameter.  
> >
> > This sounds very wrong. `Layout` should only contain size and align,
> > but not other stuff. `Layout` is part of libcore, and it is a lang
> > item (aka part of the Rust language, tightly coupled to the
> > compiler), so you shouldn't change that.
> >  
> 
> Yes. I actually modified the rust library for some experimental
> safety features.

`core` is tightly coupled with the version of rustc, so unless the
feature you are suggesting will likely ended up in upstream Rust, I
doubt that we can use the modification.

`Layout` is truly just for layout of the type, so don't put any
irrelevant bits there. You can experiment on designs of a new allocator
APIs, but please don't change `Layout`.

> > > However, the current implementation of GlobalAlloc
> > > (https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/allocator.rs#L13)
> > > is not actually used,
> > > because the kernel allocation directly resorts to `__rust_alloc`
> > > (
> > > https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/allocator.rs\#L34),
> > > `__rust_dealloc`, etc.  
> >
> > This is a complicated. Rust will call `__rust_alloc` for all
> > allocations that use the global allocator. Rust will also generate
> > `__rust_alloc` automatically if the target is a binary, `.a` or
> > `.so`, but it wouldn't generate it for `.o`.
> >
> > So we currently manually implement these functions. Perhaps we
> > should redirect `__rust_alloc` to `<KernelAllocator as
> > GlobalAlloc>::alloc`, but since `alloc` also just calls
> > GlobalAlloc>`bindings::krealloc` it does not
> > matter much.
> >  
> 
> Because I do not use `kmalloc` as a backend, I probably need to find
> alternative ways to
> enable the usage of GlobalAlloc. It would be better if I can find a
> way to pass layout as parameter
> instead of the size and align parameter for `__rust_alloc`.


Just replace the code of `__rust_alloc` with
`KernelAllocator.alloc(Layout::from_size_align_unchecked(size,
align))`, or whatever other type that implements `GlobalAlloc`.

> > > While implementing an alloc crate, I intend to pass custom
> > > parameters and extra parameters such as
> > > memory flags (GFP_KERNEL, GFP_ATOMIC, etc) through the `Layout`
> > > parameter.  
> >
> > Back to start. Why do you need to want to specify these through
> > `Layout`? As for the flags, we've discussed about this topic ~1
> > month ago in the meeting
> > (https://lore.kernel.org/rust-for-linux/alpine.DEB.2.11.2104241558400.11174@titan.ldpreload.com/)
> > weeks ago in the meeting, and we've concluded that one should
> > invoke kmalloc themselves and use unsafe APIs to construct any
> > collections if they want sepcial flags (e.g. GFP_DMA).
> >  
> 
> memory flags are one of my concerns. I am also passing some parameters
> for demonstrating
> experimental security features.


Why are you trying to use `GlobalAlloc` though? Have you looked into
the `Allocator` API? There's nothing preventing you to add a flag there:
```
struct MyAlloc(u32);

impl Allocator for MyAlloc { /*blah*/ }

Box::try_new_in(blah, MyAlloc(myflag))
```

> 
> My implementation actually does not rely on `kmalloc`, the memory
> allocation can be
> managed in Rust independently and does not need to be handled by
> kernel slab.
> 
> > A quick grep shows that in drivers/ there are 30k GFP_'s, and 27k of
> > them are GFP_KERNEL, 3k being GFP_ATOMIC, just 1k for the rest. If
> > possible we want to automatically detect the context and use
> > GFP_ATOMIC in interrupt contexts (see notes), but if it's not
> > possible probably we only need an `core::alloc::Alloc` that does
> > GFP_ATOMIC and do not need to have the allocator support arbitrary
> > flags.
> >  
> 
> Preventing the sleep-in-atomic-context bugs is also one of my
> concerns. For the memory allocation occured in rust, we can
> automatically adjust the flags through some new abstractions like
> something called `atomic_context` that takes as input a clouse.
> Although we still need to come up with a way to deal with sleepale
> APIs.
> 

I had a thought about that recently and I believe that we can probably
make use of static analysis (or Rust pluggable lints) to handle
sleepable APIs.

Please use "Reply All" next time. You didn't CC the mailing list.

- Gary