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  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]
Date:   Tue, 14 Nov 2017 14:35:23 -0800
From:   Yonghong Song <yhs@...com>
To:     Oleg Nesterov <oleg@...hat.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/17 7:51 AM, Oleg Nesterov wrote:
> On 11/13, Yonghong Song wrote:
>>
>> +static int push_setup_xol_ops(struct arch_uprobe *auprobe, struct insn *insn)
>> +{
>> +	u8 opc1 = OPCODE1(insn), reg_offset = 0;
>> +
>> +	if (opc1 < 0x50 || opc1 > 0x57)
>> +		return -ENOSYS;
>> +
>> +	if (insn->length > 2)
>> +		return -ENOSYS;
>> +#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().

I printed out some statistics. On x86_64 platform, for 32bit 
application, test_thread_flag(TIF_ADDR32) returns true and 
is_64bit_mm(mm) returns false. For 64bit application,
test_thread_flag(TIF_ADDR32) returns false and is_64bit_mm(mm) return 
true. So that is why my patch works fine.

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.

> 
> 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.

> 
> 
> 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.
> 
> I didn't have time for this problem so far. But emulation should work, so
> you can hopefully test your patch.
> 
> Oleg.
> 

Powered by blists - more mailing lists