ptrace: Convert ptrace_attach() to use lock guards

Created as testing for the conditional guard infrastructure.
Specifically this makes use of the following form:

  scoped_cond_guard (mutex_intr, return -ERESTARTNOINTR,
		     &task->signal->cred_guard_mutex) {
    ...
  }
  ...
  return 0;

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Oleg Nesterov <oleg@redhat.com>
Link: https://lkml.kernel.org/r/20231102110706.568467727%40infradead.org

diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index a23af225c8983903b103e18c52520acb13e89407..4f3dca3535568b8c9bfd614da359785300cfcb3d 100644 (file)
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -226,4 +226,6 @@ static inline void task_unlock(struct task_struct *p)
        spin_unlock(&p->alloc_lock);
 }
 
+DEFINE_GUARD(task_lock, struct task_struct *, task_lock(_T), task_unlock(_T))
+
 #endif /* _LINUX_SCHED_TASK_H */

diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index ceb56b39c70f775a736ad2918a2d0901af240f40..90bc853cafb6aeedd433d3016f17a86086df817e 100644 (file)
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -548,5 +548,31 @@ DEFINE_LOCK_GUARD_1(spinlock_irqsave, spinlock_t,
 DEFINE_LOCK_GUARD_1_COND(spinlock_irqsave, _try,
                         spin_trylock_irqsave(_T->lock, _T->flags))
 
+DEFINE_LOCK_GUARD_1(read_lock, rwlock_t,
+                   read_lock(_T->lock),
+                   read_unlock(_T->lock))
+
+DEFINE_LOCK_GUARD_1(read_lock_irq, rwlock_t,
+                   read_lock_irq(_T->lock),
+                   read_unlock_irq(_T->lock))
+
+DEFINE_LOCK_GUARD_1(read_lock_irqsave, rwlock_t,
+                   read_lock_irqsave(_T->lock, _T->flags),
+                   read_unlock_irqrestore(_T->lock, _T->flags),
+                   unsigned long flags)
+
+DEFINE_LOCK_GUARD_1(write_lock, rwlock_t,
+                   write_lock(_T->lock),
+                   write_unlock(_T->lock))
+
+DEFINE_LOCK_GUARD_1(write_lock_irq, rwlock_t,
+                   write_lock_irq(_T->lock),
+                   write_unlock_irq(_T->lock))
+
+DEFINE_LOCK_GUARD_1(write_lock_irqsave, rwlock_t,
+                   write_lock_irqsave(_T->lock, _T->flags),
+                   write_unlock_irqrestore(_T->lock, _T->flags),
+                   unsigned long flags)
+
 #undef __LINUX_INSIDE_SPINLOCK_H
 #endif /* __LINUX_SPINLOCK_H */

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index d8b5e13a2229f4ab4f6aed871cd702dbea4e01aa..5c579fb9a5e3d99eeb77587d818528b0040b6801 100644 (file)
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -386,6 +386,34 @@ static int check_ptrace_options(unsigned long data)
        return 0;
 }
 
+static inline void ptrace_set_stopped(struct task_struct *task)
+{
+       guard(spinlock)(&task->sighand->siglock);
+
+       /*
+        * If the task is already STOPPED, set JOBCTL_TRAP_STOP and
+        * TRAPPING, and kick it so that it transits to TRACED.  TRAPPING
+        * will be cleared if the child completes the transition or any
+        * event which clears the group stop states happens.  We'll wait
+        * for the transition to complete before returning from this
+        * function.
+        *
+        * This hides STOPPED -> RUNNING -> TRACED transition from the
+        * attaching thread but a different thread in the same group can
+        * still observe the transient RUNNING state.  IOW, if another
+        * thread's WNOHANG wait(2) on the stopped tracee races against
+        * ATTACH, the wait(2) may fail due to the transient RUNNING.
+        *
+        * The following task_is_stopped() test is safe as both transitions
+        * in and out of STOPPED are protected by siglock.
+        */
+       if (task_is_stopped(task) &&
+           task_set_jobctl_pending(task, JOBCTL_TRAP_STOP | JOBCTL_TRAPPING)) {
+               task->jobctl &= ~JOBCTL_STOPPED;
+               signal_wake_up_state(task, __TASK_STOPPED);
+       }
+}
+
 static int ptrace_attach(struct task_struct *task, long request,
                         unsigned long addr,
                         unsigned long flags)
@@ -393,17 +421,17 @@ static int ptrace_attach(struct task_struct *task, long request,
        bool seize = (request == PTRACE_SEIZE);
        int retval;
 
-       retval = -EIO;
        if (seize) {
                if (addr != 0)
-                       goto out;
+                       return -EIO;
                /*
                 * This duplicates the check in check_ptrace_options() because
                 * ptrace_attach() and ptrace_setoptions() have historically
                 * used different error codes for unknown ptrace options.
                 */
                if (flags & ~(unsigned long)PTRACE_O_MASK)
-                       goto out;
+                       return -EIO;
+
                retval = check_ptrace_options(flags);
                if (retval)
                        return retval;
@@ -414,88 +442,54 @@ static int ptrace_attach(struct task_struct *task, long request,
 
        audit_ptrace(task);
 
-       retval = -EPERM;
        if (unlikely(task->flags & PF_KTHREAD))
-               goto out;
+               return -EPERM;
        if (same_thread_group(task, current))
-               goto out;
+               return -EPERM;
 
        /*
         * Protect exec's credential calculations against our interference;
         * SUID, SGID and LSM creds get determined differently
         * under ptrace.
         */
-       retval = -ERESTARTNOINTR;
-       if (mutex_lock_interruptible(&task->signal->cred_guard_mutex))
-               goto out;
+       scoped_cond_guard (mutex_intr, return -ERESTARTNOINTR,
+                          &task->signal->cred_guard_mutex) {
 
-       task_lock(task);
-       retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
-       task_unlock(task);
-       if (retval)
-               goto unlock_creds;
+               scoped_guard (task_lock, task) {
+                       retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH_REALCREDS);
+                       if (retval)
+                               return retval;
+               }
 
-       write_lock_irq(&tasklist_lock);
-       retval = -EPERM;
-       if (unlikely(task->exit_state))
-               goto unlock_tasklist;
-       if (task->ptrace)
-               goto unlock_tasklist;
+               scoped_guard (write_lock_irq, &tasklist_lock) {
+                       if (unlikely(task->exit_state))
+                               return -EPERM;
+                       if (task->ptrace)
+                               return -EPERM;
 
-       task->ptrace = flags;
+                       task->ptrace = flags;
 
-       ptrace_link(task, current);
+                       ptrace_link(task, current);
 
-       /* SEIZE doesn't trap tracee on attach */
-       if (!seize)
-               send_sig_info(SIGSTOP, SEND_SIG_PRIV, task);
+                       /* SEIZE doesn't trap tracee on attach */
+                       if (!seize)
+                               send_sig_info(SIGSTOP, SEND_SIG_PRIV, task);
 
-       spin_lock(&task->sighand->siglock);
+                       ptrace_set_stopped(task);
+               }
+       }
 
        /*
-        * If the task is already STOPPED, set JOBCTL_TRAP_STOP and
-        * TRAPPING, and kick it so that it transits to TRACED.  TRAPPING
-        * will be cleared if the child completes the transition or any
-        * event which clears the group stop states happens.  We'll wait
-        * for the transition to complete before returning from this
-        * function.
-        *
-        * This hides STOPPED -> RUNNING -> TRACED transition from the
-        * attaching thread but a different thread in the same group can
-        * still observe the transient RUNNING state.  IOW, if another
-        * thread's WNOHANG wait(2) on the stopped tracee races against
-        * ATTACH, the wait(2) may fail due to the transient RUNNING.
-        *
-        * The following task_is_stopped() test is safe as both transitions
-        * in and out of STOPPED are protected by siglock.
+        * We do not bother to change retval or clear JOBCTL_TRAPPING
+        * if wait_on_bit() was interrupted by SIGKILL. The tracer will
+        * not return to user-mode, it will exit and clear this bit in
+        * __ptrace_unlink() if it wasn't already cleared by the tracee;
+        * and until then nobody can ptrace this task.
         */
-       if (task_is_stopped(task) &&
-           task_set_jobctl_pending(task, JOBCTL_TRAP_STOP | JOBCTL_TRAPPING)) {
-               task->jobctl &= ~JOBCTL_STOPPED;
-               signal_wake_up_state(task, __TASK_STOPPED);
-       }
-
-       spin_unlock(&task->sighand->siglock);
-
-       retval = 0;
-unlock_tasklist:
-       write_unlock_irq(&tasklist_lock);
-unlock_creds:
-       mutex_unlock(&task->signal->cred_guard_mutex);
-out:
-       if (!retval) {
-               /*
-                * We do not bother to change retval or clear JOBCTL_TRAPPING
-                * if wait_on_bit() was interrupted by SIGKILL. The tracer will
-                * not return to user-mode, it will exit and clear this bit in
-                * __ptrace_unlink() if it wasn't already cleared by the tracee;
-                * and until then nobody can ptrace this task.
-                */
-               wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT, TASK_KILLABLE);
-               proc_ptrace_connector(task, PTRACE_ATTACH);
-       }
+       wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT, TASK_KILLABLE);
+       proc_ptrace_connector(task, PTRACE_ATTACH);
 
-       return retval;
+       return 0;
 }
 
 /**