linux-kernel - Re: Q: user_enable_single_step() && update

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: <50197994.4090800@linutronix.de>
Date:	Wed, 01 Aug 2012 20:46:44 +0200
From:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
To:	Oleg Nesterov <oleg@...hat.com>
CC:	Roland McGrath <roland@...k.frob.com>,
	"H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
	ananth@...ibm.com, a.p.zijlstra@...llo.nl, mingo@...hat.com,
	srikar@...ux.vnet.ibm.com
Subject: Re: Q: user_enable_single_step() && update_debugctlmsr()

On 08/01/2012 05:14 PM, Oleg Nesterov wrote:
> On 08/01, Sebastian Andrzej Siewior wrote:
>>
>> On 08/01/2012 05:01 PM, Oleg Nesterov wrote:
>>> On 08/01, Sebastian Andrzej Siewior wrote:
>>>> So a patch like
>>>> --- a/arch/x86/kernel/step.c
>>>> +++ b/arch/x86/kernel/step.c
>>>> @@ -173,8 +173,8 @@ static void enable_step(struct task_struct *child,
>>>> bool block)
>>>>                   unsigned long debugctl = get_debugctlmsr();
>>>>
>>>>                   debugctl |= DEBUGCTLMSR_BTF;
>>>> -               update_debugctlmsr(debugctl);
>>>>                   set_tsk_thread_flag(child, TIF_BLOCKSTEP);
>>>> +               update_debugctlmsr(debugctl);
>>>>           } else if (test_tsk_thread_flag(child, TIF_BLOCKSTEP)) {
>>>>                   unsigned long debugctl = get_debugctlmsr();
>>>>
>>>> should fix the race
>>>
>>> No, I don't think it can fix something ;) or make any difference.
>>
>> Why? You _first_ set the task flag
>
> Yes, and this task is "child".
>
>> followed by the CPU register. Now
>> switch_to() would see the bit set and act.
>
> child sleeps and doesn't participate in switch_to(). Debugger and another
> (unrelated) task do.

This is confusing.

In order to allow the debugger to ptrace()->enable_blockstep() the
child has to be stopped/traced. We switch X86_EFLAGS_TF in child's regs
and enable DEBUGCTLMSR_BTF for the debugger which is wrong. If we quit
to userspace then the CPU on which the debugger runs has
DEBUGCTLMSR_BTF. If the tracee task runs on the same then nothing 
happens, the bit remains set.
If the tracee happens to run on a different CPU then switch_to() will
enable the DEBUGCTLMSR_BTF bit for the debugger's CPU and switch_to() 
will enable it also on the other CPU.
I added a few printks in the source and I see output that
__switch_to_xtra() enables the bit as well as in enable_single() for
debugger's CPU. I didn't find where the single step is disabled for the
tracee. I haven't notice this in __switch_to_xtra() nor or in
disable_single_step().

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