Re: [PATCH v3 1/2] livepatch: send a fake signal to all blocking tasks

From: Miroslav Benes
Date: Wed Nov 01 2017 - 11:06:28 EST



> +/*
> + * Sends a fake signal to all non-kthread tasks with TIF_PATCH_PENDING set.
> + * Kthreads with TIF_PATCH_PENDING set are woken up. Only admin can request this
> + * action currently.
> + */
> +void klp_force_signals(void)
> +{
> + struct task_struct *g, *task;
> +
> + pr_notice("signaling remaining tasks\n");
> +
> + read_lock(&tasklist_lock);
> + for_each_process_thread(g, task) {
> + if (!klp_patch_pending(task))
> + continue;
> +
> + /*
> + * There is a small race here. We could see TIF_PATCH_PENDING
> + * set and decide to wake up a kthread or send a fake signal.
> + * Meanwhile the task could migrate itself and the action
> + * would be meaningless. It is not serious though.
> + */
> + if (task->flags & PF_KTHREAD) {
> + /*
> + * Wake up a kthread which still has not been migrated.
> + */
> + wake_up_process(task);

So this is not as safe as one would hope. It tries to wake all TASK_NORMAL
tasks, which could cause headaches. Let's make it

wake_up_state(task, TASK_INTERRUPTIBLE);

to wake only kthreads sleeping interruptedly.

Thanks Petr for spotting this (offline).

Miroslav

> + } else {
> + /*
> + * Send fake signal to all non-kthread tasks which are
> + * still not migrated.
> + */
> + spin_lock_irq(&task->sighand->siglock);
> + signal_wake_up(task, 0);
> + spin_unlock_irq(&task->sighand->siglock);
> + }
> + }
> + read_unlock(&tasklist_lock);
> +}