Messages in this thread | | | From | (Eric W. Biederman) | Date | Thu, 26 Jul 2018 10:13:12 -0500 | Subject | Re: [PATCH 18/20] signal: Add calculate_sigpending() |
| |
Oleg Nesterov <oleg@redhat.com> writes:
> On 07/23, Eric W. Biederman wrote: >> >> --- a/kernel/fork.c >> +++ b/kernel/fork.c >> @@ -1988,6 +1988,7 @@ static __latent_entropy struct task_struct *copy_process( >> &p->signal->thread_head); >> } >> attach_pid(p, PIDTYPE_PID); >> + calculate_sigpending(p); > > In theory this looks racy if !CLONE_SIGHAND, please see below > >> +void calculate_sigpending(struct task_struct *new) >> +{ >> + /* Have any signals or users of TIF_SIGPENDING been delayed >> + * until after fork? >> + */ >> + bool pending = (new->jobctl & JOBCTL_PENDING_MASK) || >> + PENDING(&new->pending, &new->blocked) || >> + PENDING(&new->signal->shared_pending, &new->blocked) || >> + freezing(new) || klp_patch_pending(new); > > note that we do not hold new->sighand->siglock, but this "new" task is already > visible to find_task_by_vpid/etc; so a new signal can come right after > this check,
Good point. The localtion of the call to calculate_sigpending is wrong.
>> + update_tsk_thread_flag(new, TIF_SIGPENDING, pending); > > and then update_tsk_thread_flag() can wrongly clear TIF_SIGPENDING. > > Easy to fix, but perhaps we can simply add recalc_sigpending() into > schedule_tail() ? It already does more than just finish_task_switch/etc. > > This way we do not need the new helper (which btw can only be used by > copy_process).
The problem I have with reusing recalc_sigpending is that it does not set TIF_SIGPENDING if (freezing || klp_patch_pending).
There is obviously synergy between these two cases, I just have not figured out how to take advantage of it yet.
> Note also that either way you can remove set_tsk_thread_flag(TIF_SIGPENDING) > from ptrace_init_task().
Interesting. Yes we can remove TIF_SIGPENDING from that case because ptrace_init_task sets jobctl or queues a pending signal. I like that synergy. I like not being able to miss setting TIF_SIGPENDING during fork.
Eric
| |