On 19/04/2024 11:02 am, 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....@citrix.com>

Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com>, although ...

> ---
> Changes since v1:
>  - Fix off-by-one in range checking.
> ---
>  xen/common/livepatch_elf.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c
> index 45d73912a3cd..a67101eadc02 100644
> --- a/xen/common/livepatch_elf.c
> +++ b/xen/common/livepatch_elf.c
> @@ -4,6 +4,7 @@
>  
>  #include <xen/errno.h>
>  #include <xen/lib.h>
> +#include <xen/sections.h>
>  #include <xen/symbols.h>
>  #include <xen/livepatch_elf.h>
>  #include <xen/livepatch.h>
> @@ -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 )

... I normally vertically the (casts) in cases like this for improved
legibility.  Happy to fold on commit.

~Andrew

Reply via email to