| Date | Mon, 2 May 2022 17:39:35 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v2 07/12] ptrace: Don't change __state |
| |
On 04/29, Eric W. Biederman wrote: > > Stop playing with tsk->__state to remove TASK_WAKEKILL while a ptrace > command is executing.
Eric, I'll read this patch and the rest of this series tomorrow. Somehow I failed to force myself to read yet another version after weekend ;)
plus I don't really understand this one...
> #define TASK_KILLABLE (TASK_WAKEKILL | TASK_UNINTERRUPTIBLE) > #define TASK_STOPPED (TASK_WAKEKILL | __TASK_STOPPED) > -#define TASK_TRACED (TASK_WAKEKILL | __TASK_TRACED) > +#define TASK_TRACED __TASK_TRACED ... > static inline void signal_wake_up(struct task_struct *t, bool resume) > { > - signal_wake_up_state(t, resume ? TASK_WAKEKILL : 0); > + unsigned int state = 0; > + if (resume) { > + state = TASK_WAKEKILL; > + if (!(t->jobctl & JOBCTL_PTRACE_FROZEN)) > + state |= __TASK_TRACED; > + } > + signal_wake_up_state(t, state);
Can't understand why is this better than the previous version which removed TASK_WAKEKILL if resume... Looks a bit strange to me. But again, I didn't look at the next patches yet.
> @@ -2209,11 +2209,8 @@ static int ptrace_stop(int exit_code, int why, int clear_code, > spin_lock_irq(¤t->sighand->siglock); > } > > - /* > - * schedule() will not sleep if there is a pending signal that > - * can awaken the task. > - */ > - set_special_state(TASK_TRACED); > + if (!__fatal_signal_pending(current)) > + set_special_state(TASK_TRACED);
This is where I stuck. This probably makes sense, but what does it buy for this particular patch?
And if we check __fatal_signal_pending(), why can't ptrace_stop() simply return ?
Oleg.
|