Re: [PATCH 2/3] livepatch: Avoid blocking tasklist_lock too long

[Petr Mladek <pmladek@suse.com>]

On Thu 2025-02-13 09:32:53, Josh Poimboeuf wrote:
> On Thu, Feb 13, 2025 at 10:48:27AM +0100, Petr Mladek wrote:
> > On Wed 2025-02-12 17:36:03, Josh Poimboeuf wrote:
> > > Or, we could do something completely different.  There's no need for
> > > klp_copy_process() to copy the parent's state: a newly forked task can
> > > be patched immediately because it has no stack.
> > 
> > Is this true, please?
> > 
> > If I get it correctly then copy_process() is used also by fork(2) where
> > the child continues from fork(2) call. I can't find it in the code
> > but I suppose that the child should use a copy of the parent's stack
> > in this case.
> 
> The child's *user* stack is a copy, but the kernel stack is empty.
> 
> On x86, before adding it to the task list, copy->process() ->
> copy_thread() sets the child's kernel stack pointer to empty (pointing
> to 'struct inactive_task_frame' adjacent to user pt_regs) and sets the
> saved instruction pointer (frame->ret_addr) to 'ret_from_fork_asm'.
> 
> Then later when the child first gets scheduled, __switch_to_asm()
> switches to the new stack and pops most of the inactive_task_frame,
> except for the 'ret_from_fork_asm' return value which remains on the top
> of the stack.  Then it jumps to __switch_to() which then "returns" to
> ret_from_fork_asm().

Right. Only the *user* stack is a copy.

I guess that we really could consider the new task as migrated
and clear TIF_PATCH_PENDING.

But we can't set child->patch_state to KLP_TRANSITION_IDLE. It won't
work when the transition gets reverted. [*]

Best Regards,
Petr

[*] I gave this few brain cycles but I did not find any elegant
    way how to set this a safe way and allow using rcu_read_lock()
    in klp_try_complete_transition().

    It might be because it is Friday evening and I am leaving for
    a trip tomorrow. Also I not motivated enough to think about it
    because Yafang saw the RCU stall even with that rcu_read_lock().
    So I send this just for record.