lkml.org 
[lkml]   [2012]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCHv2 1/1] prctl: move pdeath_signal from task_struct to signal_struct
Date
There are some cases involving multi-threaded processes where pdeath_signal
(which can be set using prctl(PR_SET_PDEATHSIG, signo)) doesn't act correctly
in respect of processes vs. threads. In particular:

1) If a child process is forked from a thread of the parent process and the
child process uses prctl(PR_SET_PDEATHSIG, signo) to set pdeath_signal, then
it will receive a signal when the thread that forked it terminates instead
of the parent process itself.

2) When setting prctl(PR_SET_PDEATHSIG, signo) from a thread of the child
process, if the thread terminates before the parent process then pdeath_signal
will be cancelled as it was associated with that task only.

3) When setting prctl(PR_SET_PDEATHSIG, signo) from a thread of a process and
then querying it with prctl(PR_GET_PDEATHSIG, &signo) from another thread it
will return 0 as no pdeath_signal is not set for that task.

Documentation clearly states that PR_SET_PDEATHSIG and PR_GET_PDEATHSIG should
act on processes, in particular case #1 is very counter intuitive because the
child process should not care whether the parent is multi-threaded or not.

This patch moves pdeath_signal from task_struct to signal_struct in order to
make it effectively per-process even on multi-threaded processes.

Signed-off-by: Filipe Brandenburger <filbranden@gmail.com>
---
fs/exec.c | 2 +-
include/linux/sched.h | 7 ++++++-
kernel/cred.c | 2 +-
kernel/exit.c | 8 +++++---
kernel/fork.c | 1 -
kernel/sys.c | 5 +++--
security/apparmor/domain.c | 2 +-
security/selinux/hooks.c | 2 +-
security/smack/smack_lsm.c | 2 +-
9 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index a79786a..67ce5ee 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1151,7 +1151,7 @@ void setup_new_exec(struct linux_binprm * bprm)
/* install the new credentials */
if (!uid_eq(bprm->cred->uid, current_euid()) ||
!gid_eq(bprm->cred->gid, current_egid())) {
- current->pdeath_signal = 0;
+ current->signal->pdeath_signal = 0;
} else {
would_dump(bprm, bprm->file);
if (bprm->interp_flags & BINPRM_FLAGS_ENFORCE_NONDUMP)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f34437e..de22765 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -554,6 +554,12 @@ struct signal_struct {
unsigned int flags; /* see SIGNAL_* flags below */

/*
+ * Support for PR_SET_PDEATHSIG.
+ * If non-zero, it contains the signal sent when the parent dies.
+ */
+ int pdeath_signal;
+
+ /*
* PR_SET_CHILD_SUBREAPER marks a process, like a service
* manager, to re-parent orphan (double-forking) child processes
* to this process instead of 'init'. The service manager is
@@ -1285,7 +1291,6 @@ struct task_struct {
/* task state */
int exit_state;
int exit_code, exit_signal;
- int pdeath_signal; /* The signal sent when the parent dies */
unsigned int jobctl; /* JOBCTL_*, siglock protected */
/* ??? */
unsigned int personality;
diff --git a/kernel/cred.c b/kernel/cred.c
index de728ac..128f46e 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -496,7 +496,7 @@ int commit_creds(struct cred *new)
!cap_issubset(new->cap_permitted, old->cap_permitted)) {
if (task->mm)
set_dumpable(task->mm, suid_dumpable);
- task->pdeath_signal = 0;
+ task->signal->pdeath_signal = 0;
smp_wmb();
}

diff --git a/kernel/exit.c b/kernel/exit.c
index 34867cc..bcb471e 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -770,6 +770,11 @@ static void reparent_leader(struct task_struct *father, struct task_struct *p,
if (same_thread_group(p->real_parent, father))
return;

+ /* Check if prctl(PR_SET_PDEATHSIG, ...) was set for this process. */
+ if (p->signal->pdeath_signal)
+ group_send_sig_info(p->signal->pdeath_signal,
+ SEND_SIG_NOINFO, p);
+
/* We don't want people slaying init. */
p->exit_signal = SIGCHLD;

@@ -806,9 +811,6 @@ static void forget_original_parent(struct task_struct *father)
BUG_ON(t->ptrace);
t->parent = t->real_parent;
}
- if (t->pdeath_signal)
- group_send_sig_info(t->pdeath_signal,
- SEND_SIG_NOINFO, t);
} while_each_thread(p, t);
reparent_leader(father, p, &dead_children);
}
diff --git a/kernel/fork.c b/kernel/fork.c
index ab5211b..4c9f9b8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1402,7 +1402,6 @@ static struct task_struct *copy_process(unsigned long clone_flags,
else
p->exit_signal = (clone_flags & CSIGNAL);

- p->pdeath_signal = 0;
p->exit_state = 0;

p->nr_dirtied = 0;
diff --git a/kernel/sys.c b/kernel/sys.c
index 9ff89cb..fd9ee49 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2007,11 +2007,12 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
error = -EINVAL;
break;
}
- me->pdeath_signal = arg2;
+ me->signal->pdeath_signal = arg2;
error = 0;
break;
case PR_GET_PDEATHSIG:
- error = put_user(me->pdeath_signal, (int __user *)arg2);
+ error = put_user(me->signal->pdeath_signal,
+ (int __user *)arg2);
break;
case PR_GET_DUMPABLE:
error = get_dumpable(me->mm);
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index b81ea10..0eac0d7 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -564,7 +564,7 @@ void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
(unconfined(new_cxt->profile)))
return;

- current->pdeath_signal = 0;
+ current->signal->pdeath_signal = 0;

/* reset soft limits and set hard limits for the new profile */
__aa_transition_rlimits(profile, new_cxt->profile);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 372ec65..e839600 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2195,7 +2195,7 @@ static void selinux_bprm_committing_creds(struct linux_binprm *bprm)
flush_unauthorized_files(bprm->cred, current->files);

/* Always clear parent death signal on SID transitions. */
- current->pdeath_signal = 0;
+ current->signal->pdeath_signal = 0;

/* Check whether the new SID can inherit resource limits from the old
* SID. If not, reset all soft limits to the lower of the current
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ee0bb57..3c85790 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -496,7 +496,7 @@ static void smack_bprm_committing_creds(struct linux_binprm *bprm)
struct task_smack *bsp = bprm->cred->security;

if (bsp->smk_task != bsp->smk_forked)
- current->pdeath_signal = 0;
+ current->signal->pdeath_signal = 0;
}

/**
--
1.7.7.6


\
 
 \ /
  Last update: 2012-06-12 04:01    [W:0.342 / U:0.348 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site