linux-kernel - Re: [PATCH v2 2/5] x86/uprobes: implement x86 specific arch_uprobe_*

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: <502A0C43.2000906@linutronix.de>
Date:	Tue, 14 Aug 2012 10:28:51 +0200
From:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:	Oleg Nesterov <oleg@...hat.com>
CC:	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	linux-kernel@...r.kernel.org, x86@...nel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
	Roland McGrath <roland@...hat.com>,
	Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
	stan_shebs@...tor.com
Subject: Re: [PATCH v2 2/5] x86/uprobes: implement x86 specific arch_uprobe_*_step

On 08/13/2012 03:24 PM, Oleg Nesterov wrote:
> On 08/09, Sebastian Andrzej Siewior wrote:
>>
>> v1..v2: re-use auprobe->fixups for fixups
>
> Yes, but
>
>> @@ -46,6 +46,8 @@ struct arch_uprobe_task {
>>   #ifdef CONFIG_X86_64
>>   	unsigned long			saved_scratch_register;
>>   #endif
>> +#define UPROBE_CLEAR_TF			(1<<  0)
>> +	unsigned int			restore_flags;
>>   };
>
> this patch still adds restore_flags into arch_uprobe_task.

Yes, but

>>   static void prepare_fixups(struct arch_uprobe *auprobe, struct insn *insn)
>>   {
>> -	bool fix_ip = true, fix_call = false;	/* defaults */
>> +	bool fix_ip = true, fix_call = false, fix_tf = false;	/* defaults */
>>   	int reg;
>>
>>   	insn_get_opcode(insn);	/* should be a nop */
>>
>>   	switch (OPCODE1(insn)) {
>> +	case 0x9d:
>> +		/* popf */
>> +		fix_tf = true;
>> +		break;
>>   	case 0xc3:		/* ret/lret */
>>   	case 0xcb:
>>   	case 0xc2:
>> @@ -277,6 +284,8 @@ static void prepare_fixups(struct arch_uprobe *auprobe, struct insn *insn)
>>   		auprobe->fixups |= UPROBE_FIX_IP;
>>   	if (fix_call)
>>   		auprobe->fixups |= UPROBE_FIX_CALL;
>> +	if (fix_tf)
>> +		auprobe->fixups |= UPROBE_TF_CHANGES;
>>   }
>
> I won't insist, but do we really need fix_tf? "case 0x9d" could simply
> add UPROBE_TF_CHANGES.

if it is not 0x9d (in most cases) we need to decide on per-process
basis (not per-breakpoint) whether the task has gdb watching it or not.
So this code is evaluated once (by the time the breakpoint is
installed) but it may be executed two times: once with gdb and once
without it. On first execution the SIGTRAP will wakeup gdb, on the
second the SIGTRAP will terminate the program because there is no TRAP
handler installed.

> Oleg.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/