Re: [PATCH][v3] uprobes/x86: emulate push insns for uprobe on x86

On 11/14, Yonghong Song wrote:

On 11/14/17 7:51 AM, Oleg Nesterov wrote:

quoted

And test_thread_flag(TIF_ADDR32) is not right too. The caller is not
necessarily the probed task. See is_64bit_mm(mm) in arch_uprobe_analyze_insn().

I printed out some statistics. On x86_64 platform, for 32bit application,
test_thread_flag(TIF_ADDR32) returns true

See above. The caller can be 64-bit even if the probed task is 32bit. Or
vice versa.

and is_64bit_mm(mm) returns false.

This is what we need. Again, see its usage in arch_uprobe_analyze_insn()
and note that mm != current->mm.

So that is why my patch works fine.

test_thread_flag() can't work in general, see above.

I did not fully understand how to trigger "the caller is not necessarily the
probed task." So in the next revision, I will use is_64bit_mm(mm) instead.

register_for_each_vma() paths can call arch_uprobe_analyze_insn(), the task
which calls register_ has is not necessarily the task(s) we want to probe.

quoted

And again... please check if uprobe_init_insn() fails or not in this case
(32bit task does, say, "push r8"). If it fails, your V2 should be fine.

The compiler won't generated "push r8" for 32bit task since register "r8" is
not available on 32bit instruction.

And?

uprobes should be transparent even when it comes to user-space bugs. If
a 32bit app does asm(".byte 0x41, 0x50") for any reason we should either
deny to probe this insn (uprobe_init_insn() should fail), or we should
execute it out-of-line so that it will be trapped correctly.

Oleg.