lkml.org 
[lkml]   [2022]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 6.0 149/862] livepatch: fix race between fork and KLP transition
    Date
    From: Rik van Riel <riel@surriel.com>

    commit 747f7a2901174c9afa805dddfb7b24db6f65e985 upstream.

    The KLP transition code depends on the TIF_PATCH_PENDING and
    the task->patch_state to stay in sync. On a normal (forward)
    transition, TIF_PATCH_PENDING will be set on every task in
    the system, while on a reverse transition (after a failed
    forward one) first TIF_PATCH_PENDING will be cleared from
    every task, followed by it being set on tasks that need to
    be transitioned back to the original code.

    However, the fork code copies over the TIF_PATCH_PENDING flag
    from the parent to the child early on, in dup_task_struct and
    setup_thread_stack. Much later, klp_copy_process will set
    child->patch_state to match that of the parent.

    However, the parent's patch_state may have been changed by KLP loading
    or unloading since it was initially copied over into the child.

    This results in the KLP code occasionally hitting this warning in
    klp_complete_transition:

    for_each_process_thread(g, task) {
    WARN_ON_ONCE(test_tsk_thread_flag(task, TIF_PATCH_PENDING));
    task->patch_state = KLP_UNDEFINED;
    }

    Set, or clear, the TIF_PATCH_PENDING flag in the child task
    depending on whether or not it is needed at the time
    klp_copy_process is called, at a point in copy_process where the
    tasklist_lock is held exclusively, preventing races with the KLP
    code.

    The KLP code does have a few places where the state is changed
    without the tasklist_lock held, but those should not cause
    problems because klp_update_patch_state(current) cannot be
    called while the current task is in the middle of fork,
    klp_check_and_switch_task() which is called under the pi_lock,
    which prevents rescheduling, and manipulation of the patch
    state of idle tasks, which do not fork.

    This should prevent this warning from triggering again in the
    future, and close the race for both normal and reverse transitions.

    Signed-off-by: Rik van Riel <riel@surriel.com>
    Reported-by: Breno Leitao <leitao@debian.org>
    Reviewed-by: Petr Mladek <pmladek@suse.com>
    Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
    Fixes: d83a7cb375ee ("livepatch: change to a per-task consistency model")
    Cc: stable@kernel.org
    Signed-off-by: Petr Mladek <pmladek@suse.com>
    Link: https://lore.kernel.org/r/20220808150019.03d6a67b@imladris.surriel.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    kernel/livepatch/transition.c | 18 ++++++++++++++++--
    1 file changed, 16 insertions(+), 2 deletions(-)

    --- a/kernel/livepatch/transition.c
    +++ b/kernel/livepatch/transition.c
    @@ -610,9 +610,23 @@ void klp_reverse_transition(void)
    /* Called from copy_process() during fork */
    void klp_copy_process(struct task_struct *child)
    {
    - child->patch_state = current->patch_state;

    - /* TIF_PATCH_PENDING gets copied in setup_thread_stack() */
    + /*
    + * The parent process may have gone through a KLP transition since
    + * the thread flag was copied in setup_thread_stack earlier. Bring
    + * the task flag up to date with the parent here.
    + *
    + * The operation is serialized against all klp_*_transition()
    + * operations by the tasklist_lock. The only exception is
    + * klp_update_patch_state(current), but we cannot race with
    + * that because we are current.
    + */
    + if (test_tsk_thread_flag(current, TIF_PATCH_PENDING))
    + set_tsk_thread_flag(child, TIF_PATCH_PENDING);
    + else
    + clear_tsk_thread_flag(child, TIF_PATCH_PENDING);
    +
    + child->patch_state = current->patch_state;
    }

    /*

    \
     
     \ /
      Last update: 2022-10-19 11:22    [W:3.210 / U:0.328 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site