| Message ID | 20220317110854.39050-3-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | livepatch: fix handling of (some) relocations | expand |
On 17.03.2022 12:08, Roger Pau Monne wrote: > Track whether symbols belong to ignored sections in order to avoid > applying relocations referencing those symbols. The address of such > symbols won't be resolved and thus the relocation will likely fail or > write garbage to the destination. > > Return an error in that case, as leaving unresolved relocations would > lead to malfunctioning payload code. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with one nit (which can likely be addressed while committing): > --- a/xen/arch/arm/arm32/livepatch.c > +++ b/xen/arch/arm/arm32/livepatch.c > @@ -272,6 +272,13 @@ int arch_livepatch_perform(struct livepatch_elf *elf, > elf->name, symndx); > return -EINVAL; > } > + else if ( elf->sym[symndx].ignored ) > + { > + printk(XENLOG_ERR LIVEPATCH > + "%s: Relocation against ignored symbol %s cannot be resolved\n", > + elf->name, elf->sym[symndx].name); Indentation here (and in the other two instances mirroring this) is off by one. Jan
On Thu, Mar 17, 2022 at 02:26:50PM +0100, Jan Beulich wrote: > On 17.03.2022 12:08, Roger Pau Monne wrote: > > Track whether symbols belong to ignored sections in order to avoid > > applying relocations referencing those symbols. The address of such > > symbols won't be resolved and thus the relocation will likely fail or > > write garbage to the destination. > > > > Return an error in that case, as leaving unresolved relocations would > > lead to malfunctioning payload code. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > with one nit (which can likely be addressed while committing): > > > --- a/xen/arch/arm/arm32/livepatch.c > > +++ b/xen/arch/arm/arm32/livepatch.c > > @@ -272,6 +272,13 @@ int arch_livepatch_perform(struct livepatch_elf *elf, > > elf->name, symndx); > > return -EINVAL; > > } > > + else if ( elf->sym[symndx].ignored ) > > + { > > + printk(XENLOG_ERR LIVEPATCH > > + "%s: Relocation against ignored symbol %s cannot be resolved\n", > > + elf->name, elf->sym[symndx].name); > > Indentation here (and in the other two instances mirroring this) > is off by one. Oh, thanks, sorry for not noticing. Roger.
> From: Roger Pau Monne <roger.pau@citrix.com> > Sent: Thursday, March 17, 2022 11:08 AM > To: xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org> > Cc: Roger Pau Monne <roger.pau@citrix.com>; Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; Ross Lagerwall <ross.lagerwall@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; Julien Grall <julien@xen.org>; Bertrand Marquis <bertrand.marquis@arm.com>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Jan Beulich <jbeulich@suse.com>; Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu <wl@xen.org> > Subject: [PATCH 2/2] livepatch: avoid relocations referencing ignored section symbols > > Track whether symbols belong to ignored sections in order to avoid > applying relocations referencing those symbols. The address of such > symbols won't be resolved and thus the relocation will likely fail or > write garbage to the destination. > > Return an error in that case, as leaving unresolved relocations would > lead to malfunctioning payload code. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c index 5a06467008..6aed227818 100644 --- a/xen/arch/arm/arm32/livepatch.c +++ b/xen/arch/arm/arm32/livepatch.c @@ -272,6 +272,13 @@ int arch_livepatch_perform(struct livepatch_elf *elf, elf->name, symndx); return -EINVAL; } + else if ( elf->sym[symndx].ignored ) + { + printk(XENLOG_ERR LIVEPATCH + "%s: Relocation against ignored symbol %s cannot be resolved\n", + elf->name, elf->sym[symndx].name); + return -EINVAL; + } val = elf->sym[symndx].sym->st_value; /* S */ diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c index 6ec8dc60f0..655ded33d2 100644 --- a/xen/arch/arm/arm64/livepatch.c +++ b/xen/arch/arm/arm64/livepatch.c @@ -270,6 +270,13 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf, elf->name, symndx); return -EINVAL; } + else if ( elf->sym[symndx].ignored ) + { + printk(XENLOG_ERR LIVEPATCH + "%s: Relocation against ignored symbol %s cannot be resolved\n", + elf->name, elf->sym[symndx].name); + return -EINVAL; + } val = elf->sym[symndx].sym->st_value + r->r_addend; /* S+A */ diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index 37c9b8435e..a928e5bfcd 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -262,6 +262,13 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf, elf->name, symndx); return -EINVAL; } + else if ( elf->sym[symndx].ignored ) + { + printk(XENLOG_ERR LIVEPATCH + "%s: Relocation against ignored symbol %s cannot be resolved\n", + elf->name, elf->sym[symndx].name); + return -EINVAL; + } val = r->r_addend + elf->sym[symndx].sym->st_value; diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c index b089cacb1c..45d73912a3 100644 --- a/xen/common/livepatch_elf.c +++ b/xen/common/livepatch_elf.c @@ -334,7 +334,13 @@ int livepatch_elf_resolve_symbols(struct livepatch_elf *elf) } if ( livepatch_elf_ignore_section(elf->sec[idx].sec) ) + { + dprintk(XENLOG_DEBUG, LIVEPATCH + "%s: Symbol %s from section %s ignored\n", + elf->name, elf->sym[i].name, elf->sec[idx].name); + elf->sym[i].ignored = true; break; + } st_value += (unsigned long)elf->sec[idx].load_addr; if ( elf->sym[i].name ) diff --git a/xen/include/xen/livepatch_elf.h b/xen/include/xen/livepatch_elf.h index 5b1ec469da..7116deaddc 100644 --- a/xen/include/xen/livepatch_elf.h +++ b/xen/include/xen/livepatch_elf.h @@ -22,6 +22,7 @@ struct livepatch_elf_sec { struct livepatch_elf_sym { const Elf_Sym *sym; const char *name; + bool ignored; }; struct livepatch_elf {
Track whether symbols belong to ignored sections in order to avoid applying relocations referencing those symbols. The address of such symbols won't be resolved and thus the relocation will likely fail or write garbage to the destination. Return an error in that case, as leaving unresolved relocations would lead to malfunctioning payload code. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/arm/arm32/livepatch.c | 7 +++++++ xen/arch/arm/arm64/livepatch.c | 7 +++++++ xen/arch/x86/livepatch.c | 7 +++++++ xen/common/livepatch_elf.c | 6 ++++++ xen/include/xen/livepatch_elf.h | 1 + 5 files changed, 28 insertions(+)