On Tue, Sep 02, 2025 at 11:21:48AM +0100, Mark Brown wrote:
diff --git a/kernel/fork.c b/kernel/fork.c index af673856499d..d484ebeded33 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1907,6 +1907,51 @@ static bool need_futex_hash_allocate_default(u64 clone_flags) return true; } +static int shstk_validate_clone(struct task_struct *p,
struct kernel_clone_args *args)+{
- struct mm_struct *mm;
- struct vm_area_struct *vma;
- struct page *page;
- unsigned long addr;
- int ret;
- if (!IS_ENABLED(CONFIG_ARCH_HAS_USER_SHADOW_STACK))
return 0;- if (!args->shadow_stack_token)
return 0;- mm = get_task_mm(p);
- if (!mm)
return -EFAULT;
In theory, I don't think we need the get_task_mm() -> mmget() since copy_mm() early on already did this and the task can't disappear from underneath while we are creating it.
- mmap_read_lock(mm);
- addr = untagged_addr_remote(mm, args->shadow_stack_token);
- page = get_user_page_vma_remote(mm, addr, FOLL_FORCE | FOLL_WRITE,
&vma);
However, I wonder whether it makes sense to use the remote mm access here at all. Does this code ever run without CLONE_VM? If not, this is all done within the current mm context.
I can see the x86 shstk_alloc_thread_stack() returns early if !CLONE_VM. Similarly on arm64. I think the behaviour is preserved with this series but I'm not entirely sure from the contextual diff (I need to apply the patches locally).
Otherwise the patch looks fine (well, even the above wouldn't fail, I just find it strange that we pretend it's a remote mm but on the default allocation path like alloc_gcs() we go for current->mm).
BTW, if you repost, it might be worth cross-posting to linux-arm-kernel for wider exposure as not everyone reads LKML (and you can drop Szabolcs, his arm address is no longer valid).