Re: [PATCHv2 bpf-next 2/8] bpf: Add user memory kfuncs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: bpf_user_vm_mmap() clears active TS_COMPAT on TIF_ADDR32 tasks
- [Medium] bpf: bpf_user_vm_mmap() triggers false in_32bit_syscall() on x86_64
--

commit f91c9a5c8eeb0a10c48d0c7030bd2cd248f6d3fd
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Add user memory kfuncs

This commit adds the sleepable kfuncs bpf_copy_to_user(),
bpf_user_anon_mmap(), and bpf_user_munmap() for KPROBE BPF programs to
modify a user task's address space and manage private anonymous mappings.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 58e357302850e..ae7dfa58ae95c 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c

[ ... ]

> @@ -3741,6 +3747,110 @@ __bpf_kfunc int bpf_copy_from_user_mm(void *dst, u32 dst__sz,

[ ... ]

> +/* bpf_wq and bpf_task_work callbacks can run without a usable mm. */
> +static bool bpf_user_mm_ok(void)
> +{
> +	return current->mm && !(current->flags & (PF_KTHREAD | PF_EXITING));
> +}
> +
> +#ifdef CONFIG_IA32_EMULATION
> +static unsigned long bpf_user_vm_mmap(unsigned long len, unsigned long prot)
> +{
> +	struct thread_info *ti = current_thread_info();
> +	bool compat = test_thread_flag(TIF_ADDR32);
> +	unsigned long addr;
> +
> +	/*
> +	 * We are not in a syscall, but x86 get_unmapped_area() paths
> +	 * ignore TIF_ADDR32 and rely on in_32bit_syscall().
> +	 */
> +	if (compat)
> +		ti->status |= TS_COMPAT;
> +	addr = vm_mmap(NULL, 0, len, prot, MAP_PRIVATE | MAP_ANONYMOUS, 0);

[Severity: Medium]
As reported by bot+bpf-ci@kernel.org on v1 [1], on x86_64 kernels with
CONFIG_X86_X32_ABI=y, will calling bpf_user_anon_mmap() from a sleepable
uprobe in a native 64-bit process falsely trigger in_32bit_syscall() and
restrict mappings to the 32-bit compat range below 4 GiB?

When a 64-bit task enters a uprobe breakpoint exception (or sys_uprobe /
sys_uretprobe), task_pt_regs(current)->orig_ax is -1. Under
CONFIG_X86_X32_ABI=y, in_32bit_syscall() checks in_x32_syscall():

arch/x86/include/asm/compat.h:in_x32_syscall() {
#ifdef CONFIG_X86_X32_ABI
	if (task_pt_regs(current)->orig_ax & __X32_SYSCALL_BIT)
		return true;
#endif
	return false;
}

Because bpf_user_vm_mmap() passes addr = 0 to vm_mmap(),
arch_get_unmapped_area_topdown() and arch_get_unmapped_area() call
get_mmap_base() and find_start_end(), which both check in_32bit_syscall()
and select mm->mmap_compat_base and task_size_32bit() instead of the 64-bit
address space.

> +	if (compat)
> +		ti->status &= ~TS_COMPAT;

[Severity: Medium]
Could clearing TS_COMPAT unconditionally whenever TIF_ADDR32 is set clobber
an active 32-bit syscall's TS_COMPAT state?

When a 32-bit IA32 task enters a syscall via syscall_32_enter(), TS_COMPAT
is set in current_thread_info()->status and must remain set until
arch_exit_to_user_mode_prepare() clears it after signal handling and
syscall restart fixups:

arch/x86/include/asm/entry-common.h:arch_exit_to_user_mode_prepare() {
	...
	/*
	 * Compat syscalls set TS_COMPAT.  Make sure we clear it before
	 * returning to user mode.  We need to clear it *after* signal
	 * handling, because syscall restart has a fixup for compat
	 * syscalls.  The fixup is exercised by the ptrace_syscall_32
	 * selftest.
	...
	 */
	current_thread_info()->status &= ~(TS_COMPAT | TS_I386_REGS_POKED);
}

If a kprobe schedules a sleepable bpf_task_work callback that runs during
syscall exit (for example via task_work_run() inside get_signal() or
ptrace_notify()) and calls bpf_user_anon_mmap() -> bpf_user_vm_mmap(),
ti->status &= ~TS_COMPAT will clear TS_COMPAT before signal handling
finishes. Subsequent calls to syscall_get_error(), in_ia32_syscall(), and
in_compat_syscall() will then treat the active 32-bit syscall as a 64-bit
syscall.

Should bpf_user_vm_mmap() only set and clear TS_COMPAT when
!(ti->status & TS_COMPAT)?

[1] https://lore.kernel.org/r/571f32a410a85199bc9bcecfe07a9a9b6ef425b2052643e64031dd246c13691e@mail.kernel.org

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