| Message ID | d3af9457-d905-3e94-06e9-3d346a5a5275@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/p2m-pt: fix (latent) page table mapping leak on do_recalc() error paths | expand |
On 11/25/19 1:49 PM, Jan Beulich wrote: > There are two mappings active in the middle of do_recalc(), and hence > commit 0d0f4d78e5d1 ("p2m: change write_p2m_entry to return an error > code") should have added (or otherwise invoked) unmapping code just > like it did in p2m_next_level(), despite us not expecting any errors > here. Arrange for the existing unmap invocation to take effect in all > cases. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: George Dunlap <george.dunlap@citrix.com>
On 25.11.19 15:39, George Dunlap wrote: > On 11/25/19 1:49 PM, Jan Beulich wrote: >> There are two mappings active in the middle of do_recalc(), and hence >> commit 0d0f4d78e5d1 ("p2m: change write_p2m_entry to return an error >> code") should have added (or otherwise invoked) unmapping code just >> like it did in p2m_next_level(), despite us not expecting any errors >> here. Arrange for the existing unmap invocation to take effect in all >> cases. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: George Dunlap <george.dunlap@citrix.com> > Release-acked-by: Juergen Gross <jgross@suse.com> Juergen
--- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -391,21 +391,22 @@ static int do_recalc(struct p2m_domain * if ( err ) { ASSERT_UNREACHABLE(); - goto out; + break; } } remainder -= 1UL << ((level - 1) * PAGETABLE_ORDER); } smp_wmb(); - clear_recalc(l1, e); - err = p2m->write_p2m_entry(p2m, gfn, pent, e, level + 1); - if ( err ) + if ( !err ) { - ASSERT_UNREACHABLE(); - goto out; + clear_recalc(l1, e); + err = p2m->write_p2m_entry(p2m, gfn, pent, e, level + 1); + ASSERT(!err); } } unmap_domain_page((void *)((unsigned long)pent & PAGE_MASK)); + if ( unlikely(err) ) + goto out; } pent = p2m_find_entry(table, &gfn_remainder, gfn,
There are two mappings active in the middle of do_recalc(), and hence commit 0d0f4d78e5d1 ("p2m: change write_p2m_entry to return an error code") should have added (or otherwise invoked) unmapping code just like it did in p2m_next_level(), despite us not expecting any errors here. Arrange for the existing unmap invocation to take effect in all cases. Signed-off-by: Jan Beulich <jbeulich@suse.com>