public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ptrace: add ability to get/set signal-blocked mask
@ 2013-04-22  9:53 Andrey Vagin
       [not found] ` <20130422145704.GA30029@redhat.com>
  0 siblings, 1 reply; 4+ messages in thread
From: Andrey Vagin @ 2013-04-22  9:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: Andrey Vagin, Roland McGrath, Oleg Nesterov, Andrew Morton,
	Michael Kerrisk, Pavel Emelyanov, Cyrill Gorcunov

crtools uses a parasite code for dumping processes. The parasite code is
injected into a process with help PTRACE_SEIZE.

Currently crtools blocks signals from a parasite code. If a process has
pending signals, crtools wait while a process handles these signals.

This method is not suitable for stopped tasks. A stopped task can have a
few pending signals, when we will try to execute a parasite code, we
will need to drop SIGSTOP, but all other signals must remain pending,
because a state of processes must not be changed during checkpointing.

This patch adds two ptrace commands to set/get signal-blocked mask.

I think gdb can use this commands too.

Cc: Roland McGrath <roland@redhat.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michael Kerrisk <mtk.manpages@gmail.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Andrey Vagin <avagin@openvz.org>
---
 include/linux/signal.h      |  1 +
 include/uapi/linux/ptrace.h |  2 ++
 kernel/ptrace.c             | 36 ++++++++++++++++++++++++++++++++++++
 kernel/signal.c             |  2 +-
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/linux/signal.h b/include/linux/signal.h
index a2dcb94..706b88d 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -246,6 +246,7 @@ extern int do_sigtimedwait(const sigset_t *, siginfo_t *,
 extern int sigprocmask(int, sigset_t *, sigset_t *);
 extern void set_current_blocked(sigset_t *);
 extern void __set_current_blocked(const sigset_t *);
+extern void __set_task_blocked(struct task_struct *tsk, const sigset_t *newset);
 extern int show_unhandled_signals;
 extern int sigsuspend(sigset_t *);
 
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index 52ebcc8..1527684 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -54,6 +54,8 @@
 #define PTRACE_LISTEN		0x4208
 
 #define PTRACE_PEEKSIGINFO	0x4209
+#define PTRACE_GETSIGMASK	0x420a
+#define PTRACE_SETSIGMASK	0x420b
 
 struct ptrace_peeksiginfo_args {
 	__u64 off;	/* from which siginfo to start */
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 17ae54d..317a737 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -841,6 +841,42 @@ int ptrace_request(struct task_struct *child, long request,
 			ret = ptrace_setsiginfo(child, &siginfo);
 		break;
 
+	case PTRACE_GETSIGMASK:
+		if (addr != sizeof(sigset_t)) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (copy_to_user(datavp, &child->blocked, sizeof(sigset_t)))
+			ret = -EFAULT;
+		else
+			ret = 0;
+
+		break;
+
+	case PTRACE_SETSIGMASK:
+	{
+		sigset_t new_set;
+
+		if (addr != sizeof(sigset_t)) {
+			ret = -EINVAL;
+			break;
+		}
+
+		if (copy_from_user(&new_set, datavp, sizeof(sigset_t))) {
+			ret = -EFAULT;
+			break;
+		}
+
+		sigdelsetmask(&new_set, sigmask(SIGKILL)|sigmask(SIGSTOP));
+
+		spin_lock_irq(&child->sighand->siglock);
+		__set_task_blocked(child, &new_set);
+		spin_unlock_irq(&child->sighand->siglock);
+
+		ret = 0;
+		break;
+	}
 	case PTRACE_INTERRUPT:
 		/*
 		 * Stop tracee without any side-effect on signal or job
diff --git a/kernel/signal.c b/kernel/signal.c
index dd72567..d757e5b 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2522,7 +2522,7 @@ long do_no_restart_syscall(struct restart_block *param)
 	return -EINTR;
 }
 
-static void __set_task_blocked(struct task_struct *tsk, const sigset_t *newset)
+void __set_task_blocked(struct task_struct *tsk, const sigset_t *newset)
 {
 	if (signal_pending(tsk) && !thread_group_empty(tsk)) {
 		sigset_t newblocked;
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ptrace: add ability to get/set signal-blocked mask
       [not found] ` <20130422145704.GA30029@redhat.com>
@ 2013-04-22 15:07   ` Oleg Nesterov
  2013-04-23 10:59     ` Andrew Vagin
  0 siblings, 1 reply; 4+ messages in thread
From: Oleg Nesterov @ 2013-04-22 15:07 UTC (permalink / raw)
  To: Andrey Vagin
  Cc: linux-kernel, Roland McGrath, Andrew Morton, Michael Kerrisk,
	Pavel Emelyanov, Cyrill Gorcunov

On 04/22, Oleg Nesterov wrote:
>
> On 04/22, Andrey Vagin wrote:
> >
> > +	case PTRACE_SETSIGMASK:
> > +	{
> > +		sigset_t new_set;
> > +
> > +		if (addr != sizeof(sigset_t)) {
> > +			ret = -EINVAL;
> > +			break;
> > +		}
> > +
> > +		if (copy_from_user(&new_set, datavp, sizeof(sigset_t))) {
> > +			ret = -EFAULT;
> > +			break;
> > +		}
> > +
> > +		sigdelsetmask(&new_set, sigmask(SIGKILL)|sigmask(SIGSTOP));
> > +
> > +		spin_lock_irq(&child->sighand->siglock);
> > +		__set_task_blocked(child, &new_set);
> > +		spin_unlock_irq(&child->sighand->siglock);
>
> No, please don't...
>
> set_current_blocked/__set_task_blocked assume that tsk == current.
> If nothing else, note recalc_sigpending() in __set_task_blocked().
>
> I don't understand "This method is not suitable for stopped tasks"
> from the changelog, but if you really need PTRACE_SETSIGMASK just
> change ->blocked under siglock and do recalc_sigpending_tsk(child).
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

actually this is not necessary, the tracee will do recalc_sigpending()
after resume. But perhaps a comment make sense.

Oleg.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ptrace: add ability to get/set signal-blocked mask
  2013-04-22 15:07   ` Oleg Nesterov
@ 2013-04-23 10:59     ` Andrew Vagin
  2013-04-23 13:11       ` Oleg Nesterov
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Vagin @ 2013-04-23 10:59 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Andrey Vagin, linux-kernel, Roland McGrath, Andrew Morton,
	Michael Kerrisk, Pavel Emelyanov, Cyrill Gorcunov

On Mon, Apr 22, 2013 at 05:07:10PM +0200, Oleg Nesterov wrote:
> On 04/22, Oleg Nesterov wrote:
> >
> > On 04/22, Andrey Vagin wrote:
> > >
> > > +	case PTRACE_SETSIGMASK:
> > > +	{
> > > +		sigset_t new_set;
> > > +
> > > +		if (addr != sizeof(sigset_t)) {
> > > +			ret = -EINVAL;
> > > +			break;
> > > +		}
> > > +
> > > +		if (copy_from_user(&new_set, datavp, sizeof(sigset_t))) {
> > > +			ret = -EFAULT;
> > > +			break;
> > > +		}
> > > +
> > > +		sigdelsetmask(&new_set, sigmask(SIGKILL)|sigmask(SIGSTOP));
> > > +
> > > +		spin_lock_irq(&child->sighand->siglock);
> > > +		__set_task_blocked(child, &new_set);
> > > +		spin_unlock_irq(&child->sighand->siglock);
> >
> > No, please don't...
> >
> > set_current_blocked/__set_task_blocked assume that tsk == current.
> > If nothing else, note recalc_sigpending() in __set_task_blocked().

Thank you for the comment. It's my mistake.

> >
> > I don't understand "This method is not suitable for stopped tasks"

For example, a stopped process has a pending signal and this signal is
not blocked. crtools should dump its state, so that the process remains
in a stopped state with the same pending signal.

For dumping state crtools inject a parasite code with help POKE_DATA,
sets %rip on this code (PTRACE_SETREGS) and resumes the task
(PTRACE_CONT).

If signals are not blocked, the kernel starts to handle the signal after
resuming a process. It's out of our plan.

https://lkml.org/lkml/2011/7/20/138 - an example of code for injecting a
parasite code.

I hope the problem is become more clear.

> > from the changelog, but if you really need PTRACE_SETSIGMASK just
> > change ->blocked under siglock and do recalc_sigpending_tsk(child).
>                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> actually this is not necessary, the tracee will do recalc_sigpending()
> after resume. But perhaps a comment make sense.

__set_task_blocked executes retarget_shared_pending. I think it must be
called here too or am I wrong?

> 
> Oleg.
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ptrace: add ability to get/set signal-blocked mask
  2013-04-23 10:59     ` Andrew Vagin
@ 2013-04-23 13:11       ` Oleg Nesterov
  0 siblings, 0 replies; 4+ messages in thread
From: Oleg Nesterov @ 2013-04-23 13:11 UTC (permalink / raw)
  To: Andrew Vagin
  Cc: Andrey Vagin, linux-kernel, Roland McGrath, Andrew Morton,
	Michael Kerrisk, Pavel Emelyanov, Cyrill Gorcunov

On 04/23, Andrew Vagin wrote:
>
> On Mon, Apr 22, 2013 at 05:07:10PM +0200, Oleg Nesterov wrote:
>
> > >
> > > I don't understand "This method is not suitable for stopped tasks"
>
> For example, a stopped process has a pending signal and this signal is
> not blocked. crtools should dump its state,

Ah, thanks, I thought that you meant restoring doesn't work...

> > > from the changelog, but if you really need PTRACE_SETSIGMASK just
> > > change ->blocked under siglock and do recalc_sigpending_tsk(child).
> >                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> > actually this is not necessary, the tracee will do recalc_sigpending()
> > after resume. But perhaps a comment make sense.
>
> __set_task_blocked executes retarget_shared_pending. I think it must be
> called here too or am I wrong?

Yes sure, this is the main reason why set_current_blocked() exists.
We do not want to "delay" a group-wide signal.

But ptrace can delay it anyway? And, assuming that other threads are
stopped too this all doesn't matter at all, every thread does
recalc_sigpending() after resume. In short, if the debugger blocks a
signal, it should know what it does.

IOW, I hope this is not a problem, and I'd like to avoid the usage of
__set_task_blocked outside of signal.c or with tsk != current.

Oleg.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-04-23 13:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-22  9:53 [PATCH] ptrace: add ability to get/set signal-blocked mask Andrey Vagin
     [not found] ` <20130422145704.GA30029@redhat.com>
2013-04-22 15:07   ` Oleg Nesterov
2013-04-23 10:59     ` Andrew Vagin
2013-04-23 13:11       ` Oleg Nesterov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox