All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Ananth N Mavinakaynahalli <ananth@in.ibm.com>,
	stan_shebs@mentor.com, gdb-patches@sourceware.org
Subject: Re: [RFC 5/5 v2] uprobes: add global breakpoints
Date: Wed, 29 Aug 2012 17:49:52 +0200	[thread overview]
Message-ID: <20120829154952.GA29101@redhat.com> (raw)
In-Reply-To: <503BC2F1.5060003@linutronix.de>

On 08/27, Sebastian Andrzej Siewior wrote:
>
> On 08/22/2012 03:48 PM, Oleg Nesterov wrote:
>> On 08/21, Sebastian Andrzej Siewior wrote:
>>>
>>> - not putting the task in TASK_TRACED but simply halt. This would work
>>>    without a change to ptrace_attach() but the task continues on any
>>>    signal. So a signal friendly task would continue and not notice a
>>>    thing.
>>
>> TASK_KILLABLE
>
> That would help but would require a change in ptrace_attach() or
> something in gdb/strace/…

Well, I still think you should not touch ptrace_attach() at all.

> One thing I just noticed: If I don't register a handler for SIGUSR1 and
> send one to the application while it is in TASK_KILLABLE then the
> signal gets delivered.

Not really delivered... OK, it can be delivered (dequeued) before
the task sees SIGKILL, but this can be changed.

In short: in this case the task is correctly SIGKILL'ed. See sig_fatal()
in complete_signal().

> If I register a signal handler for it than it
> gets blocked and delivered once I resume the task.

Sure, if you have a handler, the signal is not fatal.

> Shouldn't it get blocked even if I don't register a handler for it?

No.

>> Am I understand correctly?
>>
>> If it was woken by PTRACE_ATTACH we set utask->skip_handler = 1 and
>> re-execute the instruction (yes, SIGTRAP, but this doesn't matter).
>> When the task hits this bp again we skip handler_chain() because it
>> was already reported.
>>
>> Yes? If yes, I don't think this can work. Suppose that the task
>> dequeues a signal before it returns to the usermode to re-execute
>> and enters the signal handler which can hit another uprobe.
>
> ach, those signals make everything complicated. I though signals are
> blocked until the single step is done

Yes, see uprobe_deny_signal().

> but my test just showed my
> something different.

I guess you missed the UTASK_SSTEP_TRAPPED logic.

But this doesn't matter. Surely we must not "block" signals _after_
the single step is done, and this is the problem.

> Okay, what now?

IMHO: don't do this ;)

> Blocking signals isn't probably a good idea.

This is bad and wrong idea, I think.

And, once again. Whatever you do, you can race with uprobe_register().
I mean, you must never expect that the task will hit the same uprobe
again, even if you are going to re-execute the same insn.

Oleg.


  reply	other threads:[~2012-08-29 15:48 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-07 16:12 uprobe: single step over uprobe & global breakpoints Sebastian Andrzej Siewior
2012-08-07 16:12 ` [PATCH 1/5] uprobes: Use a helper instead of ptrace's single step enable Sebastian Andrzej Siewior
2012-08-07 16:12 ` [PATCH 2/5] x86/uprobes: implement x86 specific arch_uprobe_*_step Sebastian Andrzej Siewior
2012-08-08 12:57   ` Oleg Nesterov
2012-08-08 13:17     ` Sebastian Andrzej Siewior
2012-08-08 14:53       ` Oleg Nesterov
2012-08-08 15:02         ` Sebastian Andrzej Siewior
2012-08-09  4:43         ` Ananth N Mavinakayanahalli
2012-08-09 17:09           ` [PATCH v2 " Sebastian Andrzej Siewior
2012-08-13 13:24             ` Oleg Nesterov
2012-08-14  8:28               ` Sebastian Andrzej Siewior
2012-08-14 14:27                 ` Oleg Nesterov
2012-08-20 10:47                   ` [PATCH v3] " Sebastian Andrzej Siewior
2012-08-22 14:03                     ` Oleg Nesterov
2012-08-22 14:11                       ` Sebastian Andrzej Siewior
2012-08-22 15:59                         ` Oleg Nesterov
2012-08-29 17:37                           ` Oleg Nesterov
2012-08-30  8:47                             ` Ananth N Mavinakayanahalli
2012-08-30 11:18                               ` [PATCH] x86/uprobes: don't disable single stepping if it was already on Sebastian Andrzej Siewior
2012-08-30 14:37                               ` [PATCH v3] x86/uprobes: implement x86 specific arch_uprobe_*_step Oleg Nesterov
2012-08-30 15:03                                 ` Ananth N Mavinakayanahalli
2012-08-30 15:11                                   ` Oleg Nesterov
2012-08-07 16:12 ` [PATCH 3/5] uprobes: remove check for uprobe variable in handle_swbp() Sebastian Andrzej Siewior
2012-08-08  9:10   ` Suzuki K. Poulose
2012-08-08  9:35     ` Sebastian Andrzej Siewior
2012-08-10  5:23       ` Suzuki K. Poulose
2012-08-08 12:58   ` Oleg Nesterov
2012-08-07 16:12 ` [PATCH 4/5] uprobes: probe definiton can only start with 'p' and '-' Sebastian Andrzej Siewior
2012-08-07 16:12 ` [RFC 5/5] uprobes: add global breakpoints Sebastian Andrzej Siewior
2012-08-08 13:14   ` Oleg Nesterov
2012-08-09 17:18     ` Sebastian Andrzej Siewior
2012-08-13 13:16       ` Oleg Nesterov
2012-08-14 11:43         ` Sebastian Andrzej Siewior
2012-08-13 11:34   ` Peter Zijlstra
2012-08-20 15:26     ` Sebastian Andrzej Siewior
2012-08-21 19:42     ` [RFC 5/5 v2] " Sebastian Andrzej Siewior
2012-08-22 13:48       ` Oleg Nesterov
2012-08-27 18:56         ` Sebastian Andrzej Siewior
2012-08-29 15:49           ` Oleg Nesterov [this message]
2012-08-30 20:42             ` Sebastian Andrzej Siewior

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=20120829154952.GA29101@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=ananth@in.ibm.com \
    --cc=bigeasy@linutronix.de \
    --cc=gdb-patches@sourceware.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=stan_shebs@mentor.com \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.