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

Josh Poimboeuf
2023-01-31T04:37:25Z

On Wed, Oct 26, 2022 at 04:41:21PM -0300, Marcos Paulo de Souza wrote:
> +++ b/include/linux/livepatch.h
> @@ -216,15 +216,26 @@ typedef int (*klp_shadow_ctor_t)(void *obj,
> void *ctor_data);
> typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data);
>
> -void *klp_shadow_get(void *obj, unsigned long id);
> -void *klp_shadow_alloc(void *obj, unsigned long id,
> - size_t size, gfp_t gfp_flags,
> - klp_shadow_ctor_t ctor, void *ctor_data);
> -void *klp_shadow_get_or_alloc(void *obj, unsigned long id,
> - size_t size, gfp_t gfp_flags,
> - klp_shadow_ctor_t ctor, void *ctor_data);
> -void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor);
> -void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor);
> +/**
> + * struct klp_shadow_type - shadow variable type used by the klp_object
> + * @id: shadow variable type indentifier

"identifier"

> diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c
> index aba44dcc0a88..64e83853891d 100644
> --- a/kernel/livepatch/shadow.c
> +++ b/kernel/livepatch/shadow.c
> @@ -63,24 +63,24 @@ struct klp_shadow {
> * klp_shadow_match() - verify a shadow variable matches given <obj, id>

"matches given <obj, type>" ?

> * @shadow: shadow variable to match
> * @obj: pointer to parent object
> - * @id: data identifier
> + * @shadow_type: type of the wanted shadow variable
> *
> * Return: true if the shadow variable matches.
> */
> static inline bool klp_shadow_match(struct klp_shadow *shadow, void *obj,
> - unsigned long id)
> + struct klp_shadow_type *shadow_type)
> {
> - return shadow->obj == obj && shadow->id == id;
> + return shadow->obj == obj && shadow->id == shadow_type->id;

"shadow_type" is redundant, can we just call it "type"?

Same comment for all other instances of 'shadow_type' throughout the
patch.

> @@ -159,22 +157,25 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id,
> * More complex setting can be done by @ctor function. But it is
> * called only when the buffer is really used (under klp_shadow_lock).
> */
> - new_shadow = kzalloc(size + sizeof(*new_shadow), gfp_flags);
> + new_shadow = kzalloc(size + sizeof(struct klp_shadow), gfp_flags);

Unnecessary change?

--
Josh