Hi Gary, thanks for taking a look. On Sun, Sep 15, 2024 at 04:28:13PM +0100, Gary Guo wrote: > On Thu, 12 Sep 2024 00:52:37 +0200 > Danilo Krummrich <dakr@xxxxxxxxxx> wrote: > > > Add a kernel specific `Allocator` trait, that in contrast to the one in > > Rust's core library doesn't require unstable features and supports GFP > > flags. > > > > Subsequent patches add the following trait implementors: `Kmalloc`, > > `Vmalloc` and `KVmalloc`. > > Hi Danilo, > > I think the current design is unsound regarding ZST. > > Let's say that `Allocator::alloc` gets called with a ZST type with > alignment of 4096. Your implementation will call into `krelloc` with > new_size of 0, which gets turned into of `kfree` of null pointer, which > is no-op. Everything is fine so far. Krealloc returns `ZERO_SIZE_PTR`, > and then implementation of `<Kmalloc as Allocator>::realloc` throws it > away and returns `NonNull::dangling`. > > Since `NonNull::dangling` is called with T=u8, this means the pointer > returns is 1, and it's invalid for ZSTs with larger alignments. Right, this interface is not meant to handle "allocations" for ZSTs. But you're right, since `alloc` is a safe function, we should return a properly aligned pointer. > > And this is unfixable even if the realloc implementation is changed. > Let's say the realloc now returns a dangling pointer that is suitable > aligned. Now let's see what happens when the `Allocator::free` is > called. `kfree` would be trying to free a Rust-side ZST pointer, but it > has no way to know that it's ZST! Right, that's why it's not valid to call `free` with dangling pointers.
