Re: [PATCH 3/3] livepatch: correctly handle altinstruction sections

["Doebel, Bjoern" <doebel@xxxxxxxxx>]

On 10.03.22 16:08, Roger Pau Monne wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you can confirm the sender and know the 
> content is safe.
>
>
>
> The current handling of altinstructions sections by the livepatch
> tools is incorrect, as on Xen those sections are part of .init and
> thus discarded after load. Correctly handle them by just ignoring, as
> it's done with other .init related sections.
>
> While there also add .data.ro_after_init section as a read-only
> section and introduce some syntactic sugar for comparing section
> names.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> I assume this handling of .altinstr* sections was inherited from Linux
> where the sections are not discarded after load in order to apply
> alternative patching to the loaded modules after boot.
> ---
>   common.c             |  7 +++++--
>   create-diff-object.c | 26 --------------------------
>   2 files changed, 5 insertions(+), 28 deletions(-)
>
> diff --git a/common.c b/common.c
> index 68a71f7..a148d8a 100644
> --- a/common.c
> +++ b/common.c
> @@ -249,19 +249,22 @@ int is_text_section(struct section *sec)
>                  (sec->sh.sh_flags & SHF_EXECINSTR));
>   }
>
> +#define SEC_MATCH(n) !strncmp(sec->name, n, strlen(n) - 1)
>   int is_rodata_section(struct section *sec)
>   {
>          return sec->sh.sh_type == SHT_PROGBITS &&
>                 !(sec->sh.sh_flags & (SHF_EXECINSTR | SHF_WRITE)) &&
> -              !strncmp(sec->name, ".rodata", 7);
> +              (SEC_MATCH(".rodata") || SEC_MATCH(".data.ro_after_init"));
>   }
>
>   int is_init_section(struct section *sec)
>   {
>          return sec->sh.sh_type == SHT_PROGBITS &&
>                 (sec->sh.sh_flags & SHF_ALLOC) &&
> -              !strncmp(sec->name, ".init", 5);
> +              (SEC_MATCH(".init") || SEC_MATCH(".text.startup") ||
> +               SEC_MATCH(".altinstr") || SEC_MATCH(".ctors"));
>   }
> +#undef SEC_MATCH
>
>   int is_debug_section(struct section *sec)
>   {
> diff --git a/create-diff-object.c b/create-diff-object.c
> index a516670..ec2afb4 100644
> --- a/create-diff-object.c
> +++ b/create-diff-object.c
> @@ -995,19 +995,6 @@ static int ex_table_group_size(struct kpatch_elf *kelf, 
> int offset)
>          return size;
>   }
>
> -static int altinstructions_group_size(struct kpatch_elf *kelf, int offset)
> -{
> -       static int size = 0;
> -       char *str;
> -       if (!size) {
> -               str = getenv("ALT_STRUCT_SIZE");
> -               size = str ? atoi(str) : 12;
> -       }
> -
> -       log_debug("altinstr_size=%d\n", size);
> -       return size;
> -}
> -
>   static int livepatch_hooks_group_size(struct kpatch_elf *kelf, int offset)
>   {
>          static int size = 0;
> @@ -1021,11 +1008,6 @@ static int livepatch_hooks_group_size(struct kpatch_elf 
> *kelf, int offset)
>          return size;
>   }
>
> -static int undefined_group_size(struct kpatch_elf *kelf, int offset)
> -{
> -       return 0;
> -}
> -
>   /*
>    * The rela groups in the .fixup section vary in size.  The beginning of each
>    * .fixup rela group is referenced by the .ex_table section. To find the size
> @@ -1099,14 +1081,6 @@ static struct special_section special_sections[] = {
>                  .name           = ".ex_table",
>                  .group_size     = ex_table_group_size,
>          },
> -       {
> -               .name           = ".altinstructions",
> -               .group_size     = altinstructions_group_size,
> -       },
> -       {
> -               .name           = ".altinstr_replacement",
> -               .group_size     = undefined_group_size,
> -       },
>          {
>                  .name           = ".livepatch.hooks.load",
>                  .group_size     = livepatch_hooks_group_size,
> --
> 2.34.1
Confirming, this solves the altsection issue I reported via 
https://lore.kernel.org/xen-devel/b74a68b038c31df4bb94a5b5e87453f5a249cfe2.1646753657.git.doebel@xxxxxxxxx/
Reviewed-by: Bjoern Doebel <doebel@xxxxxxxxx>



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879