On Fri, Apr 19, 2024 at 12:15:19PM +0200, Jan Beulich wrote:
> 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....@citrix.com>
> > ---
> > 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).
I've extended the commit message to explicitly mention the handling of
bounds for __init_{begin,end} checks. Let me know if you are not fine
with it (or maybe you expected something else?).
Thanks, Roger.
