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: <20120802130509.GA1465@redhat.com>
Date:	Thu, 2 Aug 2012 15:05:09 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
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, Sebastian Andrzej Siewior wrote:
>
> 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.

Yes, I guess you misread http://marc.info/?l=linux-kernel&m=134383196411020

> In order to allow the debugger to ptrace()->enable_blockstep() the
> child has to be stopped/traced.

Yes,

> We switch X86_EFLAGS_TF in child's regs

Yes,

> and enable DEBUGCTLMSR_BTF for the debugger which is wrong.

Yes, DEBUGCTLMSR_BTF is "global" (ok, per-cpu)

> If we quit
> to userspace then the CPU on which the debugger runs has
> DEBUGCTLMSR_BTF.

Yes, this doesn't look right too, but I meant another race.

I have no idea what DEBUGCTLMSR_BTF means without X86_EFLAGS_TF
though. And if gdb itself is TIF_SINGLESTEP'ed, it won't return
to userspace without report/schedule.

But, yes sure! this doesn't look right and this is the source of
other problems, and this is why I started this thread.

> If the tracee task runs

In the scenario I tried to describe above, the tracee does _not_ run.

gdb switches to _another_ X86_EFLAGS_TF task before the tracee is resumed.

>From the link above,

	We have the GDB process and the (stopped) tracee T. And we have
	another task X
        ^^^^^^^^^^^^^^

Oleg.

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