Re: [POC 0/7] livepatch: Make livepatch states, callbacks, and shadow variables work together

[Petr Mladek <pmladek@suse.com>]

On Thu 2024-07-25 16:40:20, Nicolai Stange wrote:
> Miroslav Benes <mbenes@suse.cz> writes:
> 
> >
> > Do we still need klp_state->data member? Now that it can be easily coupled 
> > with shadow variables, is there a reason to preserve it?

Good point. I have actually forgot the pointer completely.

> I would say yes, it could point to e.g. some lock protecting an
> associated shadow variable's usage. Or be used to conveniently pass on
> any kind of data between subsequent livepatches.

Honestly, I would prefer to remove the klp_state->data member. I can't
find a safe way how to maintain it.

The pointer is in struct klp_state. It means that any livepatch
supporting the state has its own copy of the pointer. We could copy
the value in __klp_enable_patch() but where?
Before calling klp_states_pre_patch() or after?

It might somehow work when the value is set only once (in a pre_patch
callback). But what if anyone tries to modify the pointer.
What if there are more livepatches using the state at the same time
(not using the atomic replace) and each livepatch has different
value.

From my POV, the klp_state->data pointer is a potentially dangerous
thing with not well defined semantic. IMHO, it is not worth it.

It the livepatches need a new synchronization mechanism, they
might allocate the lock in yet another shadow variable.

Do I miss anything, please?
Could you come up with a reasonable semantic, please?

Best Regards,
Petr