Re: [PATCH v3 3/4] livepatch: refuse to resolve symbols that belong to init sections

[Jan Beulich <jbeulich@xxxxxxxx>]

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.pau@xxxxxxxxxx>

In principle, as promised (and just to indicate earlier concerns were
addressed, as this is meaningless for other purposes)
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
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