On 23.04.2024 15:12, 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.
>
> Since __init_begin can alias other symbols (like _erodata for example)
> allow the force flag to override the check and resolve the symbol anyway.
>
> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
In principle, as promised (and just to indicate earlier concerns were
addressed, as this is meaningless for other purposes)
Acked-by: Jan Beulich <jbeul...@suse.com>
However, ...
> @@ -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 && !force )
> + {
> + printk(XENLOG_ERR LIVEPATCH
> + "%s: symbol %s is in init section, not resolving\n",
> + elf->name, elf->sym[i].name);
> + rc = -ENXIO;
> + break;
> + }
... wouldn't it make sense to still warn in this case when "force" is set?
Jan
