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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171114160304.GA19323@redhat.com>
Date:   Tue, 14 Nov 2017 17:03:04 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     Yonghong Song <yhs@...com>
Cc:     mingo@...nel.org, tglx@...utronix.de, peterz@...radead.org,
        linux-kernel@...r.kernel.org, x86@...nel.org,
        netdev@...r.kernel.org, ast@...com, kernel-team@...com
Subject: Re: [PATCH][v3] uprobes/x86: emulate push insns for uprobe on x86

On 11/14, Oleg Nesterov wrote:
>
> > +#ifdef CONFIG_X86_64
> > +	if (test_thread_flag(TIF_ADDR32))
> > +		return -ENOSYS;
> > +#endif
>
> No, this doesn't look right, see my previous email. You should do this
> check in the "if (insn->length == 2)" branch below, "push bp" should be
> emulated correctly.
>
> 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().
>
> 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.
>
>
> To remind, uprobes && 32-bit is broken, let me quote my another email:
>
> 	The 3rd bug means that you simply can't uprobe a 32bit task on a 64bit
> 	system, the in_compat_syscall() logic in get_unmapped_area() looks very
> 	wrong although I need to re-check.

Yes,

> I didn't have time for this problem so far. But emulation should work, so
> you can hopefully test your patch.

Ah, no, sizeof_long() is broken by the same reason, so you can't test it...

OK, I'll try to do something tomorrow, then we will see what can we do
with your patch...

But it would be nice if you can check what uprobe_init_insn() does in this
case, see above.

Oleg.