| Message ID | 20240409125046.60382-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/alternatives: fix .init section reference in _apply_alternatives() | expand |
On 09/04/2024 1:50 pm, Roger Pau Monne wrote: > The code in _apply_alternatives() will unconditionally attempt to read > __initdata_cf_clobber_{start,end} when called as part of applying alternatives > to a livepatch payload. When CET-IBT is active. In practice this narrows the impact to ADL/RPL and SPR/EMR. > That leads to a page-fault as > __initdata_cf_clobber_{start,end} living in .init section will have been > unmapped by the time a livepatch gets loaded. > > Fix by adding a check that limits the clobbering of endbr64 instructions to > boot time only. > > Fixes: 37ed5da851b8 ('x86/altcall: Optimise away endbr64 instruction where possible') > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> In hindsight I really should have tried livepatching when I was doing the IBT work, but that was also done under an embargo deadline. I'm happy to tweak the commit message on commit.
On Tue, Apr 09, 2024 at 01:56:16PM +0100, Andrew Cooper wrote: > On 09/04/2024 1:50 pm, Roger Pau Monne wrote: > > The code in _apply_alternatives() will unconditionally attempt to read > > __initdata_cf_clobber_{start,end} when called as part of applying alternatives > > to a livepatch payload. > > When CET-IBT is active. > > In practice this narrows the impact to ADL/RPL and SPR/EMR. Oh, yes, forgot to add that. > > > That leads to a page-fault as > > __initdata_cf_clobber_{start,end} living in .init section will have been > > unmapped by the time a livepatch gets loaded. > > > > Fix by adding a check that limits the clobbering of endbr64 instructions to > > boot time only. > > > > Fixes: 37ed5da851b8 ('x86/altcall: Optimise away endbr64 instruction where possible') > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > In hindsight I really should have tried livepatching when I was doing > the IBT work, but that was also done under an embargo deadline. > > I'm happy to tweak the commit message on commit. Sure, please go ahead. Thanks, Roger.
diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 21af0e825822..2e7ba6e0b833 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -326,7 +326,7 @@ static void init_or_livepatch _apply_alternatives(struct alt_instr *start, * Clobber endbr64 instructions now that altcall has finished optimising * all indirect branches to direct ones. */ - if ( force && cpu_has_xen_ibt ) + if ( force && cpu_has_xen_ibt && system_state < SYS_STATE_active ) { void *const *val; unsigned int clobbered = 0;
The code in _apply_alternatives() will unconditionally attempt to read __initdata_cf_clobber_{start,end} when called as part of applying alternatives to a livepatch payload. That leads to a page-fault as __initdata_cf_clobber_{start,end} living in .init section will have been unmapped by the time a livepatch gets loaded. Fix by adding a check that limits the clobbering of endbr64 instructions to boot time only. Fixes: 37ed5da851b8 ('x86/altcall: Optimise away endbr64 instruction where possible') Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/alternative.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)