Re: [PATCH v2 2/2] livepatch: refuse to resolve symbols that belong to init sections

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.04.2024 12:02, Roger Pau Monne wrote:
> Livepatch payloads containing symbols that belong to init sections can only
> lead to page faults later on, as by the time the livepatch is loaded init
> sections have already been freed.
> 
> Refuse to resolve such symbols and return an error instead.
> 
> Note such resolutions are only relevant for symbols that point to undefined
> sections (SHN_UNDEF), as that implies the symbol is not in the current payload
> and hence must either be a Xen or a different livepatch payload symbol.
> 
> Do not allow to resolve symbols that point to __init_begin, as that address is
> also unmapped.  On the other hand, __init_end is not unmapped, and hence allow
> resolutions against it.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> Changes since v1:
>  - Fix off-by-one in range checking.

Which means you addressed Andrew's comment while at the same time extending
the scope of ...

> @@ -310,6 +311,21 @@ int livepatch_elf_resolve_symbols(struct livepatch_elf 
> *elf)
>                      break;
>                  }
>              }
> +
> +            /*
> +             * Ensure not an init symbol.  Only applicable to Xen symbols, as
> +             * livepatch payloads don't have init sections or equivalent.
> +             */
> +            else if ( st_value >= (uintptr_t)&__init_begin &&
> +                      st_value < (uintptr_t)&__init_end )
> +            {
> +                printk(XENLOG_ERR LIVEPATCH
> +                       "%s: symbol %s is in init section, not resolving\n",
> +                       elf->name, elf->sym[i].name);

... what I raised concern about (and I had not seen any verbal reply to that,
I don't think).

Jan