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 <mgu...@redhat.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