Re: [PATCH v2 07/12] ptrace: Don't change __state

Oleg Nesterov
2022-05-02T15:39:35Z

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(&current->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.