lkl: make object_is_on_stack() check host thread stacks - #639
Conversation
tavip
left a comment
tavip
left a comment
There was a problem hiding this comment.
Could we instead have a custom object_is_on_stack for LKL by introducing something like _ARCH_HAS_OBJECT_IS_ON_STACK? After we added support for kasan we can now get the stack base and size, at least for posix hosts.
|
I actually considered this approach at first. However, after a brief look, I found that object_is_on_stack is used in many places throughout the kernel. Since LKL uses the host process's stack, the semantics of object_is_on_stack in this implementation may not be exactly the same as in the original Linux. In the driver, all uses of this function are related to DMA, so I think they can be replaced directly. Do you think replacing object_is_on_stack in non-DMA areas (such as KASAN and tracing) would be safe, or might it introduce other issues? I am not very familiar with them. |
I took a quick look and I've spotted of few places in USB and a couple other drivers that would benefit from it. We don't use USB yet, but there are some experimental projects using LKL that do (see the recent PRs) . For kasan it is currently a no-op because we don't enable KASAN_STACK. We don't use tracing yet, but even that seems to be a noop for LKL. So overall I think it is safe and it would be a net benefit. If you have time working on this it would be great to get it implemented. If not I am ok with the change as it is for now. |
clingfei
changed the title
Thanks for your reply! I have implemented it in the recent commit. |
object_is_on_stack() assumes that task_stack_page(current) describes the stack currently used for execution. That is not true for lkl: task->stack contains the LKL thread_info allocation, while kernel code actually runs on the host thread stack. As a result, stack-allocated kernel buffers on LKL are not detected as being on-stack. Callers such as blk_rq_map_kern() can then take the direct mapping path and eventually call virt_to_page() on a host stack address, which is not part of the kernel linear map. Allow architectures to override the stack range check by providing arch_object_is_on_stack() via __HAVE_ARCH_OBJECT_IS_ON_STACK, while keeping the KASAN tag reset in the generic object_is_on_stack() helper. Implement the LKL override using lkl_ops->thread_stack(), which returns the current host thread stack base and size when the host provides it. This lets all object_is_on_stack() callers handle LKL host-stack buffers correctly, without adding a block-layer-specific workaround. Signed-off-by: Cheng Lingfei <chenglingfei@foxmail.com>
2 participants
blk_rq_map_kern() relies on object_is_on_stack() to route stack-allocated buffers to the bio_copy_kern() bounce path, since a buffer on the stack cannot be mapped directly into a bio.
object_is_on_stack() tests whether the buffer lies within [task_stack_page(current), task_stack_page(current) + THREAD_SIZE). On native architectures that range is the task's real execution stack, so the test works. Under LKL it does not: threads actually execute on host pthread stacks, and object_is_on_stack only checks whether it lies in init_thread_union.thread_info.stack. A buffer placed on the stack therefore lives on the host stack, outside the thread_info range, and object_is_on_stack() returns false. The bounce path is skipped and bio_map_kern() ends up calling virt_to_page() on a host-stack address that is not part of the kernel linear map, corrupting the I/O.
Add blk_kern_needs_copy() to detect this case by checking that both the start and the last byte of the buffer are virt_addr_valid(), and force the bio_copy_kern() bounce path when they are not. The check is guarded by CONFIG_LKL and compiles to false on other builds, so non-LKL kernels are unchanged.