Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] Off-by-one error in the rollback loop leaves the failing section's 
partially applied relocations uncleared.
--

commit d297505bb4b1d833ec93725770d65ac1044abcad
Author: Petr Mladek <[email protected]>

livepatch: Clear relocations when klp_apply_object_relocs() fails

This commit introduces klp_write_object_relocs_limited() to unwind and
clear only the relocations that were successfully applied before
klp_write_object_relocs() encountered an error. This prevents partially
applied patches from leaving un-reverted changes that cause subsequent
patch operations to fail validation.

Link: https://lore.kernel.org/r/[email protected]

> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index a6796cd6b65fd..714f97fdd271a 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -342,14 +342,17 @@ int klp_apply_section_relocs(struct module *pmod, 
> Elf_Shdr *sechdrs,
[ ... ]
> +static int klp_write_object_relocs_limited(struct klp_patch *patch,
> +                                        struct klp_object *obj,
> +                                        bool apply, int limit)
>  {
>       int i, ret;
>       struct klp_modinfo *info = patch->mod->klp_info;
>  
> -     for (i = 1; i < info->hdr.e_shnum; i++) {
> +     if (!limit || limit > info->hdr.e_shnum)
> +             limit = info->hdr.e_shnum;
> +
> +     for (i = 1; i < limit; i++) {
>               Elf_Shdr *sec = info->sechdrs + i;
[ ... ]
> -             if (ret)
> +             if (ret) {
> +                     if (apply)
> +                             klp_write_object_relocs_limited(patch, obj, 
> false, i);

[Severity: Medium]
When an error occurs in klp_write_section_relocs(), the rollback is triggered
with the failing section index passed as the limit. Since the rollback loop
iterates with a strict less-than bounds check against this limit, will this
skip the rollback for any relocations that were successfully applied within
the failing section itself before it encountered the error?

It looks like this could leave partially applied relocations permanently
in memory within the failing section, causing future module loads to fail
validation.

>                       return ret;
> +             }
>       }
>  
>       return 0;
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=4

Reply via email to