[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <1462377062-16608-1-git-send-email-mguzik@redhat.com>
Date: Wed, 4 May 2016 17:51:02 +0200
From: Mateusz Guzik <mguzik@...hat.com>
To: linux-kernel@...r.kernel.org
Cc: Eric Paris <eparis@...isplace.org>,
James Morris <james.l.morris@...cle.com>,
Thomas Gleixner <tglx@...utronix.de>,
Al Viro <viro@...iv.linux.org.uk>,
"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: [PATCH] rlimit: locking tidy ups
rlimits are stored in task->signal and are guaranteed to remain valid as
long as the task struct is valid. All modifications are protected by
locking task->group_leader. Additionally changes to RLIMIT_CPU need
task->sighand.
do_prlimit takes tasklist_lock, which as a side effect gurantees stable
->sighand however, there is no need to take the lock for any limit other
than RLIMIT_CPU and even then we can get away with locking sighand itself.
proc_pid_limits takes ->sighand lock prior to accessing rlimits, but it
serves no purpose as it does not prevent modifications.
Both functions effectively always perform ->sighand != NULL check, but it
is only of concern when RLIMIT_CPU is being set. ->sighand is only cleared
when the process is reaped, so a dedicated check only makes it less likely
to access limits of a dead process.
As such, eliminate the unneeded check and:
- do_prlimit: stop taking tasklist_lock at all and only lock sighand when
necessary
- proc_pid_limits: lock group leader in order to obtain a stable copy
Signed-off-by: Mateusz Guzik <mguzik@...hat.com>
---
fs/proc/base.c | 6 ++----
kernel/sys.c | 22 ++++++++++++++--------
kernel/time/posix-cpu-timers.c | 3 +--
security/selinux/hooks.c | 4 +++-
4 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 704ae63..3d4963e 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -618,14 +618,12 @@ static int proc_pid_limits(struct seq_file *m, struct pid_namespace *ns,
struct pid *pid, struct task_struct *task)
{
unsigned int i;
- unsigned long flags;
struct rlimit rlim[RLIM_NLIMITS];
- if (!lock_task_sighand(task, &flags))
- return 0;
+ task_lock(task->group_leader);
memcpy(rlim, task->signal->rlim, sizeof(struct rlimit) * RLIM_NLIMITS);
- unlock_task_sighand(task, &flags);
+ task_unlock(task->group_leader);
/*
* print the file header
diff --git a/kernel/sys.c b/kernel/sys.c
index 89d5be4..1c8a67d 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1361,7 +1361,9 @@ int do_prlimit(struct task_struct *tsk, unsigned int resource,
struct rlimit *new_rlim, struct rlimit *old_rlim)
{
struct rlimit *rlim;
+ unsigned long flags;
int retval = 0;
+ int sighand_locked = 0;
if (resource >= RLIM_NLIMITS)
return -EINVAL;
@@ -1373,15 +1375,17 @@ int do_prlimit(struct task_struct *tsk, unsigned int resource,
return -EPERM;
}
- /* protect tsk->signal and tsk->sighand from disappearing */
- read_lock(&tasklist_lock);
- if (!tsk->sighand) {
- retval = -ESRCH;
- goto out;
+ task_lock(tsk->group_leader);
+ if (new_rlim && resource == RLIMIT_CPU &&
+ new_rlim->rlim_cur != RLIM_INFINITY) {
+ if (!lock_task_sighand(tsk, &flags)) {
+ retval = -ESRCH;
+ goto out;
+ }
+ sighand_locked = 1;
}
rlim = tsk->signal->rlim + resource;
- task_lock(tsk->group_leader);
if (new_rlim) {
/* Keep the capable check against init_user_ns until
cgroups can contain all limits */
@@ -1407,7 +1411,6 @@ int do_prlimit(struct task_struct *tsk, unsigned int resource,
if (new_rlim)
*rlim = *new_rlim;
}
- task_unlock(tsk->group_leader);
/*
* RLIMIT_CPU handling. Note that the kernel fails to return an error
@@ -1418,8 +1421,11 @@ int do_prlimit(struct task_struct *tsk, unsigned int resource,
if (!retval && new_rlim && resource == RLIMIT_CPU &&
new_rlim->rlim_cur != RLIM_INFINITY)
update_rlimit_cpu(tsk, new_rlim->rlim_cur);
+
+ if (sighand_locked)
+ unlock_task_sighand(tsk, &flags);
out:
- read_unlock(&tasklist_lock);
+ task_unlock(tsk->group_leader);
return retval;
}
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 1cafba8..fc38417 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -23,9 +23,8 @@ void update_rlimit_cpu(struct task_struct *task, unsigned long rlim_new)
{
cputime_t cputime = secs_to_cputime(rlim_new);
- spin_lock_irq(&task->sighand->siglock);
+ lockdep_assert_held(&task->sighand->siglock);
set_process_cpu_timer(task, CPUCLOCK_PROF, &cputime, NULL);
- spin_unlock_irq(&task->sighand->siglock);
}
static int check_clock(const clockid_t which_clock)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a86d537..d74b91a 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2506,8 +2506,10 @@ static void selinux_bprm_committing_creds(struct linux_binprm *bprm)
initrlim = init_task.signal->rlim + i;
rlim->rlim_cur = min(rlim->rlim_max, initrlim->rlim_cur);
}
- task_unlock(current);
+ spin_lock_irq(¤t->sighand->siglock);
update_rlimit_cpu(current, rlimit(RLIMIT_CPU));
+ spin_unlock_irq(¤t->sighand->siglock);
+ task_unlock(current);
}
}
--
1.8.3.1
Powered by blists - more mailing lists