linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Roland McGrath <roland@hack.frob.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	linux-kernel@vger.kernel.org, ananth@in.ibm.com,
	a.p.zijlstra@chello.nl, mingo@redhat.com,
	srikar@linux.vnet.ibm.com
Subject: Re: Q: user_enable_single_step() && update_debugctlmsr()
Date: Wed, 1 Aug 2012 16:31:32 +0200	[thread overview]
Message-ID: <20120801143132.GA7550@redhat.com> (raw)
In-Reply-To: <50193B5C.90404@linutronix.de>

On 08/01, Sebastian Andrzej Siewior wrote:
>
> On 08/01/2012 04:01 PM, Oleg Nesterov wrote:
>> On 08/01, Sebastian Andrzej Siewior wrote:
>>>
>>> On 08/01/2012 03:46 PM, Oleg Nesterov wrote:
>>>
>>>>>> But, worse, isn't it wrong? Suppose that debugger switches to
>>>>>> another TIF_SINGLESTEP&&    !TIF_BLOCKSTEP task, in this case
>>>>>> we "leak" DEBUGCTLMSR_BTF, no?
>>>>>
>>>>> __switch_to_xtra() should notice the difference in the TIF_BLOCKSTEP
>>>>> flag and disable it.
>>>>
>>>> And how it can notice the difference if there is no difference?
>>>>
>>>> (unless, of course debugger is TIF_BLOCKSTEP'ed).
>>>
>>> Yes. enable_step() sets DEBUGCTLMSR_BTF along with TIF_BLOCKSTEP.
>>> kprobes checks the same flag before touching DEBUGCTLMSR_BTF.
>>
>> It seems that you replied to the wrong email or I am confused ;)
>
> No I think I replied to the correct one :)
> enable_step() is the only place for ptrace/debugger which is touching
> DEBUGCTLMSR_BTF. It always sets DEBUGCTLMSR_BTF and TIF_BLOCKSTEP in
> sync so why should they both end up different? And once
> __switch_to_extra() notices that TIF_BLOCKSTEP from the previous task
> is different from the next task is different, then the CPU flag has
> to be changed.

OK, I was confuse by "kprobes" above.

And I think you missed my point. I'll try again.

We have the GDB process and the (stopped) tracee T. And we have
another task X which have TIF_SINGLESTEP but not TIF_BLOCKSTEP.
To simplify, suppose that X is already TASK_RUNNING but not on rq.

GDB does ptrace(PTRACE_SINGLEBLOCK, T). This sets X->TIF_BLOCKSTEP.
Now suppose that GDB is preempted right after it does
update_debugctlmsr(), and the scheduler choses X as the next task.

Both GDB and X do not have TIF_BLOCKSTEP, so __switch_to_extra()
does not update DEBUGCTLMSR_BTF.

X returns to the user-mode with TIF_SINGLESTEP and TIF_BLOCKSTEP,
the latter is wrong.

No?

Oleg.


  reply	other threads:[~2012-08-01 14:34 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-26 15:20 [PATCH] uprobes: don't enable/disable signle step if the user did it Sebastian Andrzej Siewior
2012-07-26 17:31 ` Oleg Nesterov
2012-07-27 17:39   ` Sebastian Andrzej Siewior
2012-07-27 18:04     ` Oleg Nesterov
2012-07-26 17:38 ` Sebastian Andrzej Siewior
2012-07-30 11:06 ` Ananth N Mavinakayanahalli
2012-07-30 14:16   ` Oleg Nesterov
2012-07-30 15:15     ` Sebastian Andrzej Siewior
2012-07-31  4:01     ` Ananth N Mavinakayanahalli
2012-07-31  5:22     ` Srikar Dronamraju
2012-07-31 17:38       ` Oleg Nesterov
2012-07-31 11:52     ` [PATCH 1/2] uprobes: Use a helper instead of ptrace's single step enable Sebastian Andrzej Siewior
2012-07-31 11:52       ` [PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step Sebastian Andrzej Siewior
2012-07-31 17:51         ` Oleg Nesterov
2012-07-31 19:30           ` Sebastian Andrzej Siewior
2012-08-01 12:26             ` Oleg Nesterov
2012-08-01 13:01               ` Q: user_enable_single_step() && update_debugctlmsr() Oleg Nesterov
2012-08-01 13:32                 ` Sebastian Andrzej Siewior
2012-08-01 13:46                   ` Oleg Nesterov
2012-08-01 13:54                     ` Sebastian Andrzej Siewior
2012-08-01 14:01                       ` Oleg Nesterov
2012-08-01 14:21                         ` Sebastian Andrzej Siewior
2012-08-01 14:31                           ` Oleg Nesterov [this message]
2012-08-01 14:47                             ` Oleg Nesterov
2012-08-01 14:51                             ` Sebastian Andrzej Siewior
2012-08-01 15:01                               ` Oleg Nesterov
2012-08-01 15:12                                 ` Sebastian Andrzej Siewior
2012-08-01 15:14                                   ` Oleg Nesterov
2012-08-01 18:46                                     ` Sebastian Andrzej Siewior
2012-08-02 13:05                                       ` Oleg Nesterov
2012-08-02 13:20                                         ` Sebastian Andrzej Siewior
2012-08-02 13:24                                           ` Oleg Nesterov
2012-08-01 13:43         ` [PATCH 2/2] x86/uprobes: implement x86 specific arch_uprobe_*_step Oleg Nesterov
2012-08-02  4:58           ` Ananth N Mavinakayanahalli
2012-07-31 17:40       ` [PATCH 1/2] uprobes: Use a helper instead of ptrace's single step enable Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120801143132.GA7550@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=ananth@in.ibm.com \
    --cc=bigeasy@linutronix.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=roland@hack.frob.com \
    --cc=srikar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).