Re: [RFC v1 05/19] ptwrite uprobes: Add minimal low level support for x86

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Calling synchronize_rcu() inside a loop in arch_uprobe_clear_state() causes O(N) grace period delays while holding mmap_write_lock.
- [High] find_ptwrite_page_area bypasses mmap_min_addr security protections.
- [High] Child processes execute the parent's ptwrite page due to missing VM_WIPEONFORK/VM_DONTCOPY on the special mapping.
--

commit eb94d55c50bf768d31e8cac28283dd4258bc8a40
Author: Andi Kleen <ak@kernel.org>

ptwrite uprobes: Add minimal low level support for x86

This commit adds low-level x86 support for generating and executing
PTWRITE instructions for uprobes. It pre-generates instructions into
templates and patches them when setting up the user page.

> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> index 65a2de82ecd29..df652c56414b5 100644
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c
[ ... ]
> @@ -717,6 +721,68 @@ static struct vm_area_struct *get_uprobe_trampoline(struct mm_struct *mm, unsign
[ ... ]
> +void arch_uprobe_clear_state(struct mm_struct *mm)
> +{
> +	struct uprobes_state *state = &mm->uprobes_state;
> +	struct uprobe_ptwrite_page *ptw;
> +	struct hlist_node *n;
> +
> +	hlist_for_each_entry_safe(ptw, n, &state->head_ptwrite, node) {
> +		hlist_del_rcu(&ptw->node);
> +		synchronize_rcu();

[Severity: High]
Does calling synchronize_rcu() inside this loop cause an O(N) grace period
delay while holding the mmap_write_lock?

As shown in arch_uprobe_dup_ptwrite() below, arch_uprobe_clear_state() is
called on the failure path while both the parent and child's mmap_write_lock
are held by dup_mmap(). An attacker could potentially fork and intentionally
cause memory allocation failures to stall the parent's mmap_write_lock for
several RCU grace periods, leading to a Denial of Service.

> +		__free_page(ptw->page);
> +		kfree(ptw);
> +	}
> +}
> +
> +int arch_uprobe_dup_ptwrite(struct mm_struct *oldmm, struct mm_struct *newmm)
> +{
> +	struct uprobes_state *old_state = &oldmm->uprobes_state;
> +	struct uprobes_state *new_state = &newmm->uprobes_state;
> +	struct uprobe_ptwrite_page *ptw, *new;
> +
> +	mmap_assert_write_locked(oldmm);
> +	mmap_assert_write_locked(newmm);
[ ... ]
> +fail:
> +	arch_uprobe_clear_state(newmm);
> +	return -ENOMEM;
> +}
[ ... ]
> @@ -1102,6 +1223,508 @@ static int copy_from_vaddr(struct mm_struct *mm, unsigned long vaddr, void *dst,
[ ... ]
> +static unsigned long find_ptwrite_page_area(struct mm_struct *mm,
> +					    unsigned long vaddr)
> +{
> +	VMA_ITERATOR(vmi, mm, 0);
> +	struct vm_area_struct *vma;
> +	unsigned long low, high, prev, call_end;
> +	const unsigned long call_range = (unsigned long)INT_MAX + 1;
> +
> +	mmap_assert_write_locked(mm);
> +	if (check_add_overflow(vaddr, 5UL, &call_end))
> +		return -ENOMEM;
> +	if (call_end < call_range)
> +		low = PAGE_SIZE;
> +	else
> +		low = call_end - call_range;
> +	if (low < PAGE_SIZE)
> +		low = PAGE_SIZE;

[Severity: High]
Does hardcoding the lower bound to PAGE_SIZE bypass mmap_min_addr security
protections?

If a traced user-space process installs a probe near the bottom of its
address space, find_ptwrite_page_area() will search starting at PAGE_SIZE
(0x1000). If a gap is found, _install_special_mapping() could map the
executable ptwrite VMA near NULL, which undermines the sysctl protection
against kernel NULL-pointer dereference exploits.

> +	if (low > ULONG_MAX - (PAGE_SIZE - 1))
> +		return -ENOMEM;
> +	low = PAGE_ALIGN(low);
[ ... ]
> +static struct uprobe_ptwrite_page *
> +create_uprobe_ptwrite_page(struct mm_struct *mm, unsigned long vaddr)
> +{
> +	struct uprobe_ptwrite_page *ptw;
> +	struct vm_area_struct *vma;
> +	unsigned long area;
> +
> +	area = find_ptwrite_page_area(mm, vaddr);
> +	if (IS_ERR_VALUE(area))
> +		return NULL;
> +
> +	mmap_assert_write_locked(mm);
> +
> +	ptw = kzalloc_obj(*ptw);
> +	if (!ptw)
> +		return NULL;
> +
> +	ptw->page = alloc_page(GFP_HIGHUSER | __GFP_ZERO);
> +	if (!ptw->page) {
> +		kfree(ptw);
> +		return NULL;
> +	}
> +	ptw->vaddr = area;
> +
> +	vma = _install_special_mapping(mm, area, PAGE_SIZE,
> +			VM_READ|VM_EXEC|VM_MAYEXEC|VM_MAYREAD|VM_IO,
> +			&ptwrite_mapping);

[Severity: High]
Can child processes incorrectly execute the parent's ptwrite page because
VM_WIPEONFORK or VM_DONTCOPY are missing here?

When a traced process forks, the child's VMA will receive PTEs pointing to
the parent's physical ptwrite page. Later, arch_uprobe_dup_ptwrite()
allocates a new page for the child's metadata, but the VMA still executes
the parent's page. This can cause the child to execute invalid instructions
or zeroes.

> +	if (IS_ERR(vma)) {
> +		__free_page(ptw->page);
> +		kfree(ptw);
> +		return NULL;
> +	}
> +	return ptw;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831150651.1134594-1-ak@kernel.org?part=5