On 20.02.2026 11:05, Roger Pau Monne wrote: > Introduce an extra check and comment to ensure the outer caller has > possibly taken an extra reference on the foreign page that's about to be > removed from the p2m. Otherwise the put_page() in p2m_entry_modify() won't > be safe to do ahead of the entry being removed form the p2m and any cached > states purged. > > While there also replace the error codes for unreachable paths to use > EILSEQ. > > No functional change intended. > > Signed-off-by: Roger Pau Monné <[email protected]>
Reviewed-by: Jan Beulich <[email protected]> > --- a/xen/arch/x86/include/asm/p2m.h > +++ b/xen/arch/x86/include/asm/p2m.h > @@ -1066,7 +1066,7 @@ static inline int p2m_entry_modify(struct p2m_domain > *p2m, p2m_type_t nt, > if ( !mfn_valid(nfn) || p2m != p2m_get_hostp2m(p2m->domain) ) > { > ASSERT_UNREACHABLE(); > - return -EINVAL; > + return -EILSEQ; > } > > if ( !page_get_owner_and_reference(mfn_to_page(nfn)) ) > @@ -1088,14 +1088,26 @@ static inline int p2m_entry_modify(struct p2m_domain > *p2m, p2m_type_t nt, > break; > > case p2m_map_foreign: > - if ( !mfn_valid(ofn) || p2m != p2m_get_hostp2m(p2m->domain) ) > + { > + struct page_info *pg = mfn_valid(ofn) ? mfn_to_page(ofn) : NULL; > + unsigned long ci = pg ? ACCESS_ONCE(pg->count_info) : 0; > + > + if ( !pg || p2m != p2m_get_hostp2m(p2m->domain) || > + /* > + * Rely on the caller also holding a reference to the page, so > + * that the put_page() below doesn't cause the page to be > + * freed, as it still has to be removed from the p2m. > + */ > + (ci & PGC_count_mask) <= (ci & PGC_allocated ? 2 : 1) || > + !p2m->nr_foreign ) > { > ASSERT_UNREACHABLE(); > - return -EINVAL; > + return -EILSEQ; > } > - put_page(mfn_to_page(ofn)); > + put_page(pg); > p2m->nr_foreign--; > break; > + } > > default: > break; Unrelated to the particular change here, I wonder whether it's about time to out-of-line this ever-growing function. Jan
