Mailing List Archive: [PATCH] x86/alternatives: fix .init section reference in _apply

[PATCH] x86/alternatives: fix .init section reference in _apply_alternatives()

Apr 9, 2024, 5:50 AM

Post #1 of 3 (13 views)

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(-)

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;
--
2.44.0

Re: [PATCH] x86/alternatives: fix .init section reference in _apply_alternatives() [ In reply to ]

andrew.cooper3 at citrix

Apr 9, 2024, 5:56 AM

Post #2 of 3 (13 views)

Permalink

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.

Re: [PATCH] x86/alternatives: fix .init section reference in _apply_alternatives() [ In reply to ]

roger.pau at citrix

Apr 9, 2024, 6:00 AM

Post #3 of 3 (13 views)

Permalink

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.