[PATCH 1/2] livepatch: Fix having __klp_objects relics in non-livepatch modules

Petr Pavlu posted 2 patches 3 weeks, 5 days ago
There is a newer version of this series
[PATCH 1/2] livepatch: Fix having __klp_objects relics in non-livepatch modules
Posted by Petr Pavlu 3 weeks, 5 days ago
The linker script scripts/module.lds.S specifies that all input
__klp_objects sections should be consolidated into an output section of
the same name, and start/stop symbols should be created to enable
scripts/livepatch/init.c to locate this data.

This start/stop pattern is not ideal for modules because the symbols are
created even if no __klp_objects input sections are present.
Consequently, a dummy __klp_objects section also appears in the
resulting module. This unnecessarily pollutes non-livepatch modules.

Instead, since modules are relocatable files, the usual method for
locating consolidated data in a module is to read its section table.
This approach avoids the aforementioned problem.

The klp_modinfo already stores a copy of the entire section table with
the final addresses. Introduce a helper function that
scripts/livepatch/init.c can call to obtain the location of the
__klp_objects section from this data.

Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
 include/linux/livepatch.h |  3 +++
 kernel/livepatch/core.c   | 20 ++++++++++++++++++++
 scripts/livepatch/init.c  | 17 ++++++-----------
 scripts/module.lds.S      |  7 +------
 4 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 772919e8096a..ca90adbe89ed 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -175,6 +175,9 @@ int klp_enable_patch(struct klp_patch *);
 int klp_module_coming(struct module *mod);
 void klp_module_going(struct module *mod);
 
+struct klp_object_ext *klp_build_locate_init_objects(const struct module *mod,
+						     unsigned int *nr_objs);
+
 void klp_copy_process(struct task_struct *child);
 void klp_update_patch_state(struct task_struct *task);
 
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 9917756dae46..4e0ac47b3623 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -1356,6 +1356,26 @@ void klp_module_going(struct module *mod)
 	mutex_unlock(&klp_mutex);
 }
 
+struct klp_object_ext *klp_build_locate_init_objects(const struct module *mod,
+						     unsigned int *nr_objs)
+{
+	struct klp_modinfo *info = mod->klp_info;
+
+	for (int i = 1; i < info->hdr.e_shnum; i++) {
+		Elf_Shdr *shdr = &info->sechdrs[i];
+
+		if (strcmp(info->secstrings + shdr->sh_name, "__klp_objects"))
+			continue;
+
+		*nr_objs = shdr->sh_size / sizeof(struct klp_object_ext);
+		return (struct klp_object_ext *)shdr->sh_addr;
+	}
+
+	*nr_objs = 0;
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(klp_build_locate_init_objects);
+
 static int __init klp_init(void)
 {
 	klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
diff --git a/scripts/livepatch/init.c b/scripts/livepatch/init.c
index 2274d8f5a482..23e037d6de19 100644
--- a/scripts/livepatch/init.c
+++ b/scripts/livepatch/init.c
@@ -9,19 +9,16 @@
 #include <linux/slab.h>
 #include <linux/livepatch.h>
 
-extern struct klp_object_ext __start_klp_objects[];
-extern struct klp_object_ext __stop_klp_objects[];
-
 static struct klp_patch *patch;
 
 static int __init livepatch_mod_init(void)
 {
+	struct klp_object_ext *obj_exts;
 	struct klp_object *objs;
 	unsigned int nr_objs;
 	int ret;
 
-	nr_objs = __stop_klp_objects - __start_klp_objects;
-
+	obj_exts = klp_build_locate_init_objects(THIS_MODULE, &nr_objs);
 	if (!nr_objs) {
 		pr_err("nothing to patch!\n");
 		ret = -EINVAL;
@@ -41,7 +38,7 @@ static int __init livepatch_mod_init(void)
 	}
 
 	for (int i = 0; i < nr_objs; i++) {
-		struct klp_object_ext *obj_ext = __start_klp_objects + i;
+		struct klp_object_ext *obj_ext = obj_exts + i;
 		struct klp_func_ext *funcs_ext = obj_ext->funcs;
 		unsigned int nr_funcs = obj_ext->nr_funcs;
 		struct klp_func *funcs = objs[i].funcs;
@@ -90,12 +87,10 @@ static int __init livepatch_mod_init(void)
 
 static void __exit livepatch_mod_exit(void)
 {
-	unsigned int nr_objs;
-
-	nr_objs = __stop_klp_objects - __start_klp_objects;
+	struct klp_object *obj;
 
-	for (int i = 0; i < nr_objs; i++)
-		kfree(patch->objs[i].funcs);
+	klp_for_each_object_static(patch, obj)
+		kfree(obj->funcs);
 
 	kfree(patch->objs);
 	kfree(patch);
diff --git a/scripts/module.lds.S b/scripts/module.lds.S
index 3037d5e5527c..383d19beffb4 100644
--- a/scripts/module.lds.S
+++ b/scripts/module.lds.S
@@ -35,12 +35,7 @@ SECTIONS {
 	__patchable_function_entries : { *(__patchable_function_entries) }
 
 	__klp_funcs		0: ALIGN(8) { KEEP(*(__klp_funcs)) }
-
-	__klp_objects		0: ALIGN(8) {
-		__start_klp_objects = .;
-		KEEP(*(__klp_objects))
-		__stop_klp_objects = .;
-	}
+	__klp_objects		0: ALIGN(8) { KEEP(*(__klp_objects)) }
 
 #ifdef CONFIG_ARCH_USES_CFI_TRAPS
 	__kcfi_traps		: { KEEP(*(.kcfi_traps)) }
-- 
2.52.0
Re: [PATCH 1/2] livepatch: Fix having __klp_objects relics in non-livepatch modules
Posted by Joe Lawrence 2 weeks, 6 days ago
On Wed, Jan 14, 2026 at 01:29:53PM +0100, Petr Pavlu wrote:
> The linker script scripts/module.lds.S specifies that all input
> __klp_objects sections should be consolidated into an output section of
> the same name, and start/stop symbols should be created to enable
> scripts/livepatch/init.c to locate this data.
> 
> This start/stop pattern is not ideal for modules because the symbols are
> created even if no __klp_objects input sections are present.
> Consequently, a dummy __klp_objects section also appears in the
> resulting module. This unnecessarily pollutes non-livepatch modules.
> 
> Instead, since modules are relocatable files, the usual method for
> locating consolidated data in a module is to read its section table.
> This approach avoids the aforementioned problem.
> 
> The klp_modinfo already stores a copy of the entire section table with
> the final addresses. Introduce a helper function that
> scripts/livepatch/init.c can call to obtain the location of the
> __klp_objects section from this data.
> 
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
> ---
>  include/linux/livepatch.h |  3 +++
>  kernel/livepatch/core.c   | 20 ++++++++++++++++++++
>  scripts/livepatch/init.c  | 17 ++++++-----------
>  scripts/module.lds.S      |  7 +------
>  4 files changed, 30 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index 772919e8096a..ca90adbe89ed 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -175,6 +175,9 @@ int klp_enable_patch(struct klp_patch *);
>  int klp_module_coming(struct module *mod);
>  void klp_module_going(struct module *mod);
>  
> +struct klp_object_ext *klp_build_locate_init_objects(const struct module *mod,
> +						     unsigned int *nr_objs);
> +
>  void klp_copy_process(struct task_struct *child);
>  void klp_update_patch_state(struct task_struct *task);
>  
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 9917756dae46..4e0ac47b3623 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -1356,6 +1356,26 @@ void klp_module_going(struct module *mod)
>  	mutex_unlock(&klp_mutex);
>  }
>  
> +struct klp_object_ext *klp_build_locate_init_objects(const struct module *mod,
> +						     unsigned int *nr_objs)
> +{
> +	struct klp_modinfo *info = mod->klp_info;
> +
> +	for (int i = 1; i < info->hdr.e_shnum; i++) {
> +		Elf_Shdr *shdr = &info->sechdrs[i];
> +
> +		if (strcmp(info->secstrings + shdr->sh_name, "__klp_objects"))
> +			continue;
> +

Since this function is doing a string comparision to find the ELF
section, would it make sense to open up the API by allowing to caller to
specify the sh_name?  That would give scripts/livepatch/init.c future
flexibility in finding similarly crafted data structures.  Disregard if
there is already a pattern of doing it this way :)

> +		*nr_objs = shdr->sh_size / sizeof(struct klp_object_ext);
> +		return (struct klp_object_ext *)shdr->sh_addr;
> +	}
> +
> +	*nr_objs = 0;
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(klp_build_locate_init_objects);
> +
>  static int __init klp_init(void)
>  {
>  	klp_root_kobj = kobject_create_and_add("livepatch", kernel_kobj);
> diff --git a/scripts/livepatch/init.c b/scripts/livepatch/init.c
> index 2274d8f5a482..23e037d6de19 100644
> --- a/scripts/livepatch/init.c
> +++ b/scripts/livepatch/init.c
> @@ -9,19 +9,16 @@
>  #include <linux/slab.h>
>  #include <linux/livepatch.h>
>  
> -extern struct klp_object_ext __start_klp_objects[];
> -extern struct klp_object_ext __stop_klp_objects[];
> -
>  static struct klp_patch *patch;
>  
>  static int __init livepatch_mod_init(void)
>  {
> +	struct klp_object_ext *obj_exts;
>  	struct klp_object *objs;
>  	unsigned int nr_objs;
>  	int ret;
>  
> -	nr_objs = __stop_klp_objects - __start_klp_objects;
> -
> +	obj_exts = klp_build_locate_init_objects(THIS_MODULE, &nr_objs);
>  	if (!nr_objs) {
>  		pr_err("nothing to patch!\n");
>  		ret = -EINVAL;
> @@ -41,7 +38,7 @@ static int __init livepatch_mod_init(void)
>  	}
>  
>  	for (int i = 0; i < nr_objs; i++) {
> -		struct klp_object_ext *obj_ext = __start_klp_objects + i;
> +		struct klp_object_ext *obj_ext = obj_exts + i;
>  		struct klp_func_ext *funcs_ext = obj_ext->funcs;
>  		unsigned int nr_funcs = obj_ext->nr_funcs;
>  		struct klp_func *funcs = objs[i].funcs;
> @@ -90,12 +87,10 @@ static int __init livepatch_mod_init(void)
>  
>  static void __exit livepatch_mod_exit(void)
>  {
> -	unsigned int nr_objs;
> -
> -	nr_objs = __stop_klp_objects - __start_klp_objects;
> +	struct klp_object *obj;
>  
> -	for (int i = 0; i < nr_objs; i++)
> -		kfree(patch->objs[i].funcs);
> +	klp_for_each_object_static(patch, obj)
> +		kfree(obj->funcs);
>  
>  	kfree(patch->objs);
>  	kfree(patch);
> diff --git a/scripts/module.lds.S b/scripts/module.lds.S
> index 3037d5e5527c..383d19beffb4 100644
> --- a/scripts/module.lds.S
> +++ b/scripts/module.lds.S
> @@ -35,12 +35,7 @@ SECTIONS {
>  	__patchable_function_entries : { *(__patchable_function_entries) }
>  
>  	__klp_funcs		0: ALIGN(8) { KEEP(*(__klp_funcs)) }
> -
> -	__klp_objects		0: ALIGN(8) {
> -		__start_klp_objects = .;
> -		KEEP(*(__klp_objects))
> -		__stop_klp_objects = .;
> -	}
> +	__klp_objects		0: ALIGN(8) { KEEP(*(__klp_objects)) }
>  
>  #ifdef CONFIG_ARCH_USES_CFI_TRAPS
>  	__kcfi_traps		: { KEEP(*(.kcfi_traps)) }
> -- 
> 2.52.0
> 

Acked-by: Joe Lawrence <joe.lawrence@redhat.com>

--
Joe
Re: [PATCH 1/2] livepatch: Fix having __klp_objects relics in non-livepatch modules
Posted by Petr Pavlu 2 weeks, 5 days ago
On 1/19/26 11:19 PM, Joe Lawrence wrote:
> On Wed, Jan 14, 2026 at 01:29:53PM +0100, Petr Pavlu wrote:
>> The linker script scripts/module.lds.S specifies that all input
>> __klp_objects sections should be consolidated into an output section of
>> the same name, and start/stop symbols should be created to enable
>> scripts/livepatch/init.c to locate this data.
>>
>> This start/stop pattern is not ideal for modules because the symbols are
>> created even if no __klp_objects input sections are present.
>> Consequently, a dummy __klp_objects section also appears in the
>> resulting module. This unnecessarily pollutes non-livepatch modules.
>>
>> Instead, since modules are relocatable files, the usual method for
>> locating consolidated data in a module is to read its section table.
>> This approach avoids the aforementioned problem.
>>
>> The klp_modinfo already stores a copy of the entire section table with
>> the final addresses. Introduce a helper function that
>> scripts/livepatch/init.c can call to obtain the location of the
>> __klp_objects section from this data.
>>
>> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
>> ---
>>  include/linux/livepatch.h |  3 +++
>>  kernel/livepatch/core.c   | 20 ++++++++++++++++++++
>>  scripts/livepatch/init.c  | 17 ++++++-----------
>>  scripts/module.lds.S      |  7 +------
>>  4 files changed, 30 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
>> index 772919e8096a..ca90adbe89ed 100644
>> --- a/include/linux/livepatch.h
>> +++ b/include/linux/livepatch.h
>> @@ -175,6 +175,9 @@ int klp_enable_patch(struct klp_patch *);
>>  int klp_module_coming(struct module *mod);
>>  void klp_module_going(struct module *mod);
>>  
>> +struct klp_object_ext *klp_build_locate_init_objects(const struct module *mod,
>> +						     unsigned int *nr_objs);
>> +
>>  void klp_copy_process(struct task_struct *child);
>>  void klp_update_patch_state(struct task_struct *task);
>>  
>> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
>> index 9917756dae46..4e0ac47b3623 100644
>> --- a/kernel/livepatch/core.c
>> +++ b/kernel/livepatch/core.c
>> @@ -1356,6 +1356,26 @@ void klp_module_going(struct module *mod)
>>  	mutex_unlock(&klp_mutex);
>>  }
>>  
>> +struct klp_object_ext *klp_build_locate_init_objects(const struct module *mod,
>> +						     unsigned int *nr_objs)
>> +{
>> +	struct klp_modinfo *info = mod->klp_info;
>> +
>> +	for (int i = 1; i < info->hdr.e_shnum; i++) {
>> +		Elf_Shdr *shdr = &info->sechdrs[i];
>> +
>> +		if (strcmp(info->secstrings + shdr->sh_name, "__klp_objects"))
>> +			continue;
>> +
> 
> Since this function is doing a string comparision to find the ELF
> section, would it make sense to open up the API by allowing to caller to
> specify the sh_name?  That would give scripts/livepatch/init.c future
> flexibility in finding similarly crafted data structures.  Disregard if
> there is already a pattern of doing it this way :)

Makes sense. I'll change the function signature to:

void *klp_locate_section_objs(const struct module *mod, const char *name, size_t object_size, unsigned int *nr_objs);

-- 
Thanks,
Petr