This confuses disassemblers, at the very least. Move
.altinstr_replacement to .init.text. The previously redundant ALIGN()
now gets converted to page alignment, such that the hypervisor mapping
won't have this as executable (it'll instead get mapped r/w, which I'm
told is intended to be adjusted at some point).
Note that for the actual patching logic's purposes this part of
.init.text _has_ to live after _einittext (or before _sinittext), or
else branch_insn_requires_update() would produce wrong results.
Also, to have .altinstr_replacement have consistent attributes in the
object files, add "x" to the one instance where it was missing.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Put past _einittext.
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -148,6 +148,8 @@ SECTIONS
_sinittext = .;
*(.init.text)
_einittext = .;
+ . = ALIGN(PAGE_SIZE); /* Avoid mapping alt insns executable */
+ *(.altinstr_replacement)
} :text
. = ALIGN(PAGE_SIZE);
.init.data : {
@@ -169,8 +171,6 @@ SECTIONS
__alt_instructions = .;
*(.altinstructions)
__alt_instructions_end = .;
- . = ALIGN(4);
- *(.altinstr_replacement)
#ifdef CONFIG_DEBUG_LOCK_PROFILE
. = ALIGN(POINTER_ALIGN);
--- a/xen/include/asm-arm/alternative.h
+++ b/xen/include/asm-arm/alternative.h
@@ -67,7 +67,7 @@ int apply_alternatives(const struct alt_
ALTINSTR_ENTRY(feature,cb) \
".popsection\n" \
" .if " __stringify(cb) " == 0\n" \
- ".pushsection .altinstr_replacement, \"a\"\n" \
+ ".pushsection .altinstr_replacement, \"ax\"\n" \
"663:\n\t" \
newinstr "\n" \
"664:\n\t" \
Hi Jan, On 14/06/2021 15:52, Jan Beulich wrote: > This confuses disassemblers, at the very least. Move > .altinstr_replacement to .init.text. The previously redundant ALIGN() > now gets converted to page alignment, such that the hypervisor mapping > won't have this as executable (it'll instead get mapped r/w, which I'm > told is intended to be adjusted at some point). > > Note that for the actual patching logic's purposes this part of > .init.text _has_ to live after _einittext (or before _sinittext), or > else branch_insn_requires_update() would produce wrong results. > > Also, to have .altinstr_replacement have consistent attributes in the > object files, add "x" to the one instance where it was missing. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers, > --- > v2: Put past _einittext. > > --- a/xen/arch/arm/xen.lds.S > +++ b/xen/arch/arm/xen.lds.S > @@ -148,6 +148,8 @@ SECTIONS > _sinittext = .; > *(.init.text) > _einittext = .; > + . = ALIGN(PAGE_SIZE); /* Avoid mapping alt insns executable */ > + *(.altinstr_replacement) > } :text > . = ALIGN(PAGE_SIZE); > .init.data : { > @@ -169,8 +171,6 @@ SECTIONS > __alt_instructions = .; > *(.altinstructions) > __alt_instructions_end = .; > - . = ALIGN(4); > - *(.altinstr_replacement) > > #ifdef CONFIG_DEBUG_LOCK_PROFILE > . = ALIGN(POINTER_ALIGN); > --- a/xen/include/asm-arm/alternative.h > +++ b/xen/include/asm-arm/alternative.h > @@ -67,7 +67,7 @@ int apply_alternatives(const struct alt_ > ALTINSTR_ENTRY(feature,cb) \ > ".popsection\n" \ > " .if " __stringify(cb) " == 0\n" \ > - ".pushsection .altinstr_replacement, \"a\"\n" \ > + ".pushsection .altinstr_replacement, \"ax\"\n" \ > "663:\n\t" \ > newinstr "\n" \ > "664:\n\t" \ > -- Julien Grall
On 14/06/2021 15:54, Julien Grall wrote: > Hi Jan, > > On 14/06/2021 15:52, Jan Beulich wrote: >> This confuses disassemblers, at the very least. Move >> .altinstr_replacement to .init.text. The previously redundant ALIGN() >> now gets converted to page alignment, such that the hypervisor mapping >> won't have this as executable (it'll instead get mapped r/w, which I'm >> told is intended to be adjusted at some point). >> >> Note that for the actual patching logic's purposes this part of >> .init.text _has_ to live after _einittext (or before _sinittext), or >> else branch_insn_requires_update() would produce wrong results. >> >> Also, to have .altinstr_replacement have consistent attributes in the >> object files, add "x" to the one instance where it was missing. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Julien Grall <jgrall@amazon.com> Comitted. Cheers, -- Julien Grall
© 2016 - 2024 Red Hat, Inc.