Re: [PATCH 3/4] livepatch/shadow: Introduce klp_shadow_type structure

[Petr Mladek <pmladek@suse.com>]

On Thu 2022-08-25 10:50:20, Joe Lawrence wrote:
> On 7/1/22 3:48 PM, Marcos Paulo de Souza wrote:
> > The shadow variable type will be used in klp_shadow_alloc/get/free
> > functions instead of id/ctor/dtor parameters. As a result, all callers
> > use the same callbacks consistently[*][**].
> > 
> > The structure will be used in the next patch that will manage the
> > lifetime of shadow variables and execute garbage collection automatically.
> > 
> > [*] From the user POV, it might have been easier to pass $id instead
> >     of pointer to struct klp_shadow_type.
> > 
> >     The problem is that each livepatch registers its own struct
> >     klp_shadow_type and defines its own @ctor/@dtor callbacks. It would
> >     be unclear what callback should be used. They should be compatible.
> > 
> >     This problem is gone when each livepatch explicitly uses its
> >     own struct klp_shadow_type pointing to its own callbacks.
> > 
> > [**] test_klp_shadow_vars.c uses a custom @dtor to show that it was called.
> >     The message must be disabled when called via klp_shadow_free_all()
> >     because the ordering of freed variables is not well defined there.
> >     It has to be done using another hack after switching to
> >     klp_shadow_types.
> > 
> 
> Is the ordering problem new to this patchset?  Shadow variables are
> still saved in klp_shadow_hash and I think the only change in this patch
> is that we need to compare through shadow_type and not id directly.  Or
> does patch 4/4 change behavior here?  Just curious, otherwise this patch
> is pretty straightforward.

The problem is old. klp_shadow_free_all() uses hash_for_each(). It
iterates the hashes sorted by the hash value. The tested arrays are
on stack. The address of the stack is different in every run.
As a result, the hash is always different and the pointers
are sorted in different order.

Best Regards,
Petr