On Tue, Aug 11, 2020 at 08:45:16AM +0200, Oleg Nesterov wrote:
On 08/11, Jann Horn wrote:
--- a/kernel/task_work.c +++ b/kernel/task_work.c @@ -42,7 +42,8 @@ task_work_add(struct task_struct *task, struct callback_head *work, int notify) set_notify_resume(task); break; case TWA_SIGNAL:
if (lock_task_sighand(task, &flags)) {
if (!(task->jobctl & JOBCTL_TASK_WORK) &&lock_task_sighand(task, &flags)) { task->jobctl |= JOBCTL_TASK_WORK; signal_wake_up(task, 0); unlock_task_sighand(task, &flags);I think that should work in theory, but if you want to be able to do a proper unlocked read of task->jobctl here, then I think you'd have to use READ_ONCE() here
Agreed,
and make all existing writes to ->jobctl use WRITE_ONCE().
->jobctl is always modified with ->siglock held, do we really need WRITE_ONCE() ?
In theory, yes. The compiler doesn't know about locks, it can tear writes whenever it feels like it. In practise it doesn't happen much, but...