Messages in this thread Patch in this message | | | Date | Wed, 13 Apr 2022 20:59:10 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/5] sched,ptrace: Fix ptrace_check_attach() vs PREEMPT_RT |
| |
On 04/13, Oleg Nesterov wrote: > > On 04/13, Oleg Nesterov wrote: > > > > I like 1-2 but I need to read them (and other patches) again, a > > couple of nits right now. > > Sorry, didn't have time to do this today, and now I am already sleeping. > > But... on a second thought, it seems there is a better solution. If nothing > else it is simpler and doesn't duplicate the wait_task_inactive() logic. > > How about the patch below instead? On top of 1/5. > > Yes,yes, incomplete. in particular see the "!!!!!!!!!" comments. Just to > explain the idea.
Cough. forget to attach the patch, sorry for noise.
Oleg. ---
diff --git a/include/linux/sched/jobctl.h b/include/linux/sched/jobctl.h index ec8b312f7506..1b5a57048e13 100644 --- a/include/linux/sched/jobctl.h +++ b/include/linux/sched/jobctl.h @@ -22,7 +22,8 @@ struct task_struct; #define JOBCTL_STOPPED_BIT 24 #define JOBCTL_TRACED_BIT 25 -#define JOBCTL_TRACED_FROZEN_BIT 26 +#define JOBCTL_TRACED_XXX_BIT 25 +#define JOBCTL_TRACED_FROZEN_BIT 27 #define JOBCTL_STOP_DEQUEUED (1UL << JOBCTL_STOP_DEQUEUED_BIT) #define JOBCTL_STOP_PENDING (1UL << JOBCTL_STOP_PENDING_BIT) @@ -35,6 +36,7 @@ struct task_struct; #define JOBCTL_STOPPED (1UL << JOBCTL_STOPPED_BIT) #define JOBCTL_TRACED (1UL << JOBCTL_TRACED_BIT) +#define JOBCTL_TRACED_XXX (1UL << JOBCTL_TRACED_XXX_BIT) #define JOBCTL_TRACED_FROZEN (1UL << JOBCTL_TRACED_FROZEN_BIT) #define JOBCTL_TRAP_MASK (JOBCTL_TRAP_STOP | JOBCTL_TRAP_NOTIFY) diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 626f96d275c7..86b5226e6ba2 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -255,6 +255,19 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state) { int ret = -ESRCH; + if (!(child->ptrace && child->parent == current)) + return ret; + + if (ignore_state) + return 0; + + if (wait_on_bit(&task->jobctl, JOBCTL_TRACED_XXX_BIT, TASK_KILLABLE)) + return -EINTR; + // now that the tracee cleared JOBCTL_TRACED_XXX_BIT + // wait_task_inactive() should succeed or fail "really soon". + if (!wait_task_inactive(child, __TASK_TRACED)) + return ret; + /* * We take the read lock around doing both checks to close a * possible race where someone else was tracing our child and @@ -269,23 +282,11 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state) * child->sighand can't be NULL, release_task() * does ptrace_unlink() before __exit_signal(). */ - if (ignore_state || ptrace_freeze_traced(child)) + if (ptrace_freeze_traced(child)) ret = 0; } read_unlock(&tasklist_lock); - if (!ret && !ignore_state) { - if (!wait_task_inactive(child, __TASK_TRACED)) { - /* - * This can only happen if may_ptrace_stop() fails and - * ptrace_stop() changes ->state back to TASK_RUNNING, - * so we should not worry about leaking __TASK_TRACED. - */ - WARN_ON(READ_ONCE(child->__state) == __TASK_TRACED); - ret = -ESRCH; - } - } - return ret; } diff --git a/kernel/signal.c b/kernel/signal.c index 0aea3f0a8002..5ca6235e5231 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2220,7 +2220,7 @@ static int ptrace_stop(int exit_code, int why, int clear_code, * schedule() will not sleep if there is a pending signal that * can awaken the task. */ - current->jobctl |= JOBCTL_TRACED; + current->jobctl |= (JOBCTL_TRACED | JOBCTL_TRACED_XXX); set_special_state(TASK_TRACED); /* @@ -2291,6 +2291,10 @@ static int ptrace_stop(int exit_code, int why, int clear_code, preempt_disable(); read_unlock(&tasklist_lock); cgroup_enter_frozen(); + // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + // wrong, needs siglock + current->jobctl &= ~JOBCTL_TRACED_XXX; + wake_up_bit(¤t->jobctl, ~JOBCTL_TRACED_XXX_BIT); preempt_enable_no_resched(); freezable_schedule(); cgroup_leave_frozen(true); @@ -2308,6 +2312,8 @@ static int ptrace_stop(int exit_code, int why, int clear_code, if (gstop_done) do_notify_parent_cldstop(current, false, why); + // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! + // need to clear ~JOBCTL_TRACED_XXX /* tasklist protects us from ptrace_freeze_traced() */ __set_current_state(TASK_RUNNING); read_code = false;
| |