| Message ID | 20240305121121.3527944-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/livepatch: Reinstate the ability to patch .rodata | expand |
On 05/03/2024 12:11 pm, Andrew Cooper wrote: > diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c > index d2efe9e11492..f45812483b8e 100644 > --- a/xen/common/virtual_region.c > +++ b/xen/common/virtual_region.c > @@ -91,9 +91,15 @@ void relax_virtual_region_perms(void) > > rcu_read_lock(&rcu_virtual_region_lock); > list_for_each_entry_rcu( region, &virtual_region_list, list ) > + { > modify_xen_mappings_lite((unsigned long)region->text_start, > PAGE_ALIGN((unsigned long)region->text_end), > PAGE_HYPERVISOR_RWX); > + if ( region->rodata_start ) > + modify_xen_mappings_lite((unsigned long)region->rodata_start, > + ROUNDUP((unsigned long)region->rodata_end, PAGE_SIZE), I missed the final refresh to turn this to PAGE_ALIGN(). Fixed locally. ~Andrew
On Tue, Mar 05, 2024 at 01:02:37PM +0000, Andrew Cooper wrote: > On 05/03/2024 12:11 pm, Andrew Cooper wrote: > > diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c > > index d2efe9e11492..f45812483b8e 100644 > > --- a/xen/common/virtual_region.c > > +++ b/xen/common/virtual_region.c > > @@ -91,9 +91,15 @@ void relax_virtual_region_perms(void) > > > > rcu_read_lock(&rcu_virtual_region_lock); > > list_for_each_entry_rcu( region, &virtual_region_list, list ) > > + { > > modify_xen_mappings_lite((unsigned long)region->text_start, > > PAGE_ALIGN((unsigned long)region->text_end), > > PAGE_HYPERVISOR_RWX); > > + if ( region->rodata_start ) > > + modify_xen_mappings_lite((unsigned long)region->rodata_start, > > + ROUNDUP((unsigned long)region->rodata_end, PAGE_SIZE), > > I missed the final refresh to turn this to PAGE_ALIGN(). Fixed locally. With that: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
On Tue, Mar 5, 2024 at 1:02 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 05/03/2024 12:11 pm, Andrew Cooper wrote: > > diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c > > index d2efe9e11492..f45812483b8e 100644 > > --- a/xen/common/virtual_region.c > > +++ b/xen/common/virtual_region.c > > @@ -91,9 +91,15 @@ void relax_virtual_region_perms(void) > > > > rcu_read_lock(&rcu_virtual_region_lock); > > list_for_each_entry_rcu( region, &virtual_region_list, list ) > > + { > > modify_xen_mappings_lite((unsigned long)region->text_start, > > PAGE_ALIGN((unsigned long)region->text_end), > > PAGE_HYPERVISOR_RWX); > > + if ( region->rodata_start ) > > + modify_xen_mappings_lite((unsigned long)region->rodata_start, > > + ROUNDUP((unsigned long)region->rodata_end, PAGE_SIZE), > > I missed the final refresh to turn this to PAGE_ALIGN(). Fixed locally. In that case, Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index ee539f001b73..4f76127e1f77 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -62,7 +62,7 @@ int arch_livepatch_safety_check(void) int noinline arch_livepatch_quiesce(void) { /* - * Relax perms on .text to be RWX, so we can modify them. + * Relax perms on .text/.rodata, so we can modify them. * * This relaxes perms globally, but all other CPUs are waiting on us. */ @@ -75,7 +75,7 @@ int noinline arch_livepatch_quiesce(void) void noinline arch_livepatch_revive(void) { /* - * Reinstate perms on .text to be RX. This also cleans out the dirty + * Reinstate perms on .text/.rodata. This also cleans out the dirty * bits, which matters when CET Shstk is active. * * The other CPUs waiting for us could in principle have re-walked while diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c index d2efe9e11492..f45812483b8e 100644 --- a/xen/common/virtual_region.c +++ b/xen/common/virtual_region.c @@ -91,9 +91,15 @@ void relax_virtual_region_perms(void) rcu_read_lock(&rcu_virtual_region_lock); list_for_each_entry_rcu( region, &virtual_region_list, list ) + { modify_xen_mappings_lite((unsigned long)region->text_start, PAGE_ALIGN((unsigned long)region->text_end), PAGE_HYPERVISOR_RWX); + if ( region->rodata_start ) + modify_xen_mappings_lite((unsigned long)region->rodata_start, + ROUNDUP((unsigned long)region->rodata_end, PAGE_SIZE), + PAGE_HYPERVISOR_RW); + } rcu_read_unlock(&rcu_virtual_region_lock); } @@ -103,9 +109,15 @@ void tighten_virtual_region_perms(void) rcu_read_lock(&rcu_virtual_region_lock); list_for_each_entry_rcu( region, &virtual_region_list, list ) + { modify_xen_mappings_lite((unsigned long)region->text_start, PAGE_ALIGN((unsigned long)region->text_end), PAGE_HYPERVISOR_RX); + if ( region->rodata_start ) + modify_xen_mappings_lite((unsigned long)region->rodata_start, + ROUNDUP((unsigned long)region->rodata_end, PAGE_SIZE), + PAGE_HYPERVISOR_RO); + } rcu_read_unlock(&rcu_virtual_region_lock); } #endif /* CONFIG_X86 */
This reinstates the capability to patch .rodata in load/unload hooks, which was lost when we stopped using CR0.WP=0 to patch. This turns out to be rather less of a large TODO than I thought at the time. Fixes: 8676092a0f16 ("x86/livepatch: Fix livepatch application when CET is active") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> CC: Ross Lagerwall <ross.lagerwall@citrix.com> CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/livepatch.c | 4 ++-- xen/common/virtual_region.c | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-)