[Xen-changelog] [xen master] xen/arm: p2m: Don't check the return of p2m_get_root_pointer() with BUG_ON()

[patchbot@xxxxxxx]

commit 31b4f4ab6634f85163656b470dffc6d974917853
Author:     Julien Grall <julien.grall@xxxxxxx>
AuthorDate: Tue Oct 15 17:10:42 2019 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Oct 31 16:19:14 2019 +0100

    xen/arm: p2m: Don't check the return of p2m_get_root_pointer() with BUG_ON()
    
    It turns out that the BUG_ON() was actually reachable with well-crafted
    hypercalls. The BUG_ON() is here to prevent catch logical error, so
    crashing Xen is a bit over the top.
    
    While all the holes should now be fixed, it would be better to downgrade
    the BUG_ON() to something less fatal to prevent any more DoS.
    
    The BUG_ON() in p2m_get_entry() is now replaced by ASSERT_UNREACHABLE()
    to catch mistake in debug build and return INVALID_MFN for production
    build. The interface also requires to set page_order to give an idea of
    the size of "hole". So 'level' is now set so we report a hole of size of
    the an entry of the root page-table. This stays inline with what happen
    when the GFN is higher than p2m->max_mapped_gfn.
    
    The BUG_ON() in p2m_resolve_translation_fault() is now replaced by
    ASSERT_UNREACHABLE() to catch mistake in debug build and just report a
    fault for producion build.
    
    This is part of XSA-301.
    
    Reported-by: Julien Grall <Julien.Grall@xxxxxxx>
    Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
    Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
---
 xen/arch/arm/p2m.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 8d20d27961..ce59f2b503 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -395,7 +395,12 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
      * the table should always be non-NULL because the gfn is below
      * p2m->max_mapped_gfn and the root table pages are always present.
      */
-    BUG_ON(table == NULL);
+    if ( !table )
+    {
+        ASSERT_UNREACHABLE();
+        level = P2M_ROOT_LEVEL;
+        goto out;
+    }
 
     for ( level = P2M_ROOT_LEVEL; level < 3; level++ )
     {
@@ -1196,7 +1201,11 @@ bool p2m_resolve_translation_fault(struct domain *d, 
gfn_t gfn)
      * The table should always be non-NULL because the gfn is below
      * p2m->max_mapped_gfn and the root table pages are always present.
      */
-    BUG_ON(table == NULL);
+    if ( !table )
+    {
+        ASSERT_UNREACHABLE();
+        goto out;
+    }
 
     /*
      * Go down the page-tables until an entry has the valid bit unset or
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog