[BUG] x86: 32-bit uprobes are broken

[Oleg Nesterov <oleg@redhat.com>]

Paulo reported that uprobing the 32-bit tasks is broken.

This script

	#!/usr/bin/bash

	echo 0 > /proc/sys/kernel/randomize_va_space

	echo 'void main(void) {}' > TEST.c

	# -fcf-protection to ensure that the 1st endbr32 insn can't be eulated
	gcc -m32 -fcf-protection=branch TEST.c -o test

	bpftrace -e 'uprobe:./test:main {}' -c ./test

"hangs", the probed ./test task enters the endless loop.

This patch

	--- a/kernel/events/uprobes.c
	+++ b/kernel/events/uprobes.c
	@@ -1710,8 +1710,11 @@ static int xol_add_vma(struct mm_struct *mm, struct xol_area *area)
	 
		if (!area->vaddr) {
			/* Try to map as high as possible, this is only a hint. */
	+		if (test_thread_flag(TIF_ADDR32))
	+			current_thread_info()->status |= TS_COMPAT;
			area->vaddr = get_unmapped_area(NULL, TASK_SIZE - PAGE_SIZE,
							PAGE_SIZE, 0, 0);
	+		current_thread_info()->status &= ~TS_COMPAT;
			if (IS_ERR_VALUE(area->vaddr)) {
				ret = area->vaddr;
				goto fail;

or this one

	--- a/arch/x86/kernel/sys_x86_64.c
	+++ b/arch/x86/kernel/sys_x86_64.c
	@@ -205,6 +205,8 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr0,
			info.low_limit = PAGE_SIZE;
	 
		info.high_limit = get_mmap_base(0);
	+	if (test_thread_flag(TIF_ADDR32))
	+		info.high_limit = current->mm->mmap_compat_base;
		if (!(filp && is_file_hugepages(filp))) {
			info.start_gap = stack_guard_placement(vm_flags);
			info.align_offset = pgoff << PAGE_SHIFT;

"fixes" the problem.

-----------------------------------------------------------------------------------------
The problem is that with randomize_va_space == 0 get_unmapped_area(TASK_SIZE - PAGE_SIZE)
called by xol_add_vma() can't just return the "addr == TASK_SIZE - PAGE_SIZE" hint, this
addr is used by the stack vma.

arch_get_unmapped_area/arch_get_unmapped_area_topdown() doesn't take TIF_ADDR32 into
account and in_32bit_syscall() is false, this leads to info.high_limit > TASK_SIZE.
get_area() happily returns the "high" address and then get_unmapped_area() returns
ENOMEM after the

	if (addr > TASK_SIZE - len)
		return -ENOMEM;

check; TASK_SIZE checks TIF_ADDR32.

handle_swbp() doesn't report this failure (probably it should) and silently restarts
the probed insn. Endless loop.

-----------------------------------------------------------------------------------------
I am considering the patch which adds something like

	// x86 version will use the TS_COMPAT hack
	unsigned long __weak arch_uprobe_area(void)
	{
		return get_unmapped_area(NULL, TASK_SIZE - PAGE_SIZE, PAGE_SIZE, 0, 0);
	}

but perhaps there is a better solution? perhaps it makes more sense to change
arch/x86/kernel/sys_x86_64.c? What is the point of ignoring TIF_ADDR32 if the
high adress will be nacked by the "if (addr > TASK_SIZE - len)" check anyway?

Oleg.