| Message ID | 5b097aa2-7ced-4971-bd6e-96618bb6f2df@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] Arm: correct FIXADDR_TOP | expand |
On 13/08/2024 13:49, Jan Beulich wrote: > > > While reviewing a RISC-V patch cloning the Arm code, I noticed an > off-by-1 here: FIX_PMAP_{BEGIN,END} being an inclusive range and > FIX_LAST being the same as FIX_PMAP_END, FIXADDR_TOP cannot derive from > FIX_LAST alone, or else the BUG_ON() in virt_to_fix() would trigger if > FIX_PMAP_END ended up being used. > > While touching this area also add a check for fixmap and boot FDT area > to not only not overlap, but to have at least one (unmapped) page in > between. > > Fixes: 4f17357b52f6 ("xen/arm: add Persistent Map (PMAP) infrastructure") > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Michal Orzel <michal.orzel@amd.com> ~Michal
Hi, On 13/08/2024 12:57, Michal Orzel wrote: > > > On 13/08/2024 13:49, Jan Beulich wrote: >> >> >> While reviewing a RISC-V patch cloning the Arm code, I noticed an >> off-by-1 here: FIX_PMAP_{BEGIN,END} being an inclusive range and >> FIX_LAST being the same as FIX_PMAP_END, FIXADDR_TOP cannot derive from >> FIX_LAST alone, or else the BUG_ON() in virt_to_fix() would trigger if >> FIX_PMAP_END ended up being used. >> >> While touching this area also add a check for fixmap and boot FDT area >> to not only not overlap, but to have at least one (unmapped) page in >> between. >> >> Fixes: 4f17357b52f6 ("xen/arm: add Persistent Map (PMAP) infrastructure") >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > Reviewed-by: Michal Orzel <michal.orzel@amd.com> Committed. Cheers,
--- a/xen/arch/arm/include/asm/fixmap.h +++ b/xen/arch/arm/include/asm/fixmap.h @@ -18,7 +18,7 @@ #define FIX_LAST FIX_PMAP_END #define FIXADDR_START FIXMAP_ADDR(0) -#define FIXADDR_TOP FIXMAP_ADDR(FIX_LAST) +#define FIXADDR_TOP FIXMAP_ADDR(FIX_LAST + 1) #ifndef __ASSEMBLY__ --- a/xen/arch/arm/mmu/setup.c +++ b/xen/arch/arm/mmu/setup.c @@ -128,6 +128,12 @@ static void __init __maybe_unused build_ #undef CHECK_SAME_SLOT #undef CHECK_DIFFERENT_SLOT + + /* + * Fixmaps must not overlap with boot FDT mapping area. Make sure there's + * at least one guard page in between. + */ + BUILD_BUG_ON(FIXADDR_TOP >= BOOT_FDT_VIRT_START); } lpae_t __init pte_of_xenaddr(vaddr_t va)
While reviewing a RISC-V patch cloning the Arm code, I noticed an off-by-1 here: FIX_PMAP_{BEGIN,END} being an inclusive range and FIX_LAST being the same as FIX_PMAP_END, FIXADDR_TOP cannot derive from FIX_LAST alone, or else the BUG_ON() in virt_to_fix() would trigger if FIX_PMAP_END ended up being used. While touching this area also add a check for fixmap and boot FDT area to not only not overlap, but to have at least one (unmapped) page in between. Fixes: 4f17357b52f6 ("xen/arm: add Persistent Map (PMAP) infrastructure") Signed-off-by: Jan Beulich <jbeulich@suse.com> --- v2: Adjust FIXADDR_TOP instead. Add BUILD_BUG_ON() as suggested by Michal.