[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 2/2] x86/dom0: only disable SMAP for the PV dom0 build
On 30.07.2024 17:28, Roger Pau Monne wrote: > The PVH dom0 builder doesn't switch page tables and has no need to run with > SMAP disabled. > > Use stac() and clac(), as that's safe to use even if events would hit in the > middle of the region with SMAP disabled. Nesting stac() and clac() calls is > not safe, so the stac() call is done strictly after elf_load_binary() because > that uses raw_{copy_to,clear}_guest() accessors which toggle SMAP. And that was the main concern causing the CR4.SMAP override to be used instead, iirc. While I'm sure you've properly audited all code paths, how can we be sure there's not going to be another stac() or clac() added somewhere? Or setting of EFLAGS as a whole, clearing EFLAGS.AC without that being explicit? I think we'd be better off sticking to the fiddling with CR4. > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> Considering the bug Andrew pointed out on the code you remove from setup.c, don't we want a Fixes: tag as well? > --- a/xen/arch/x86/pv/dom0_build.c > +++ b/xen/arch/x86/pv/dom0_build.c > @@ -830,6 +830,15 @@ int __init dom0_construct_pv(struct domain *d, > printk("Failed to load the kernel binary\n"); > goto out; > } > + > + /* > + * Disable SMAP to allow user-accesses when running on dom0 page-tables. > + * Note this must be done after elf_load_binary(), as such helper uses > + * raw_{copy_to,clear}_guest() helpers which internally call > stac()/clac() > + * and those calls would otherwise nest with the ones here. Just in case you and Andrew would outvote me on which approach to take: I'm okay with "helpers" here, but the earlier "such helper" reads a little odd to me. Imo using "that" or "it" instead would be better. Not the least because personally a function like elf_load_binary() goes beyond what I'd call a mere "helper" (in that case dom0_construct_pv() toos could be deemed a helper, etc). Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |