[Xen-devel] [PATCH] xen: netback: check if foreign pages are actually netback-created foreign pages.

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

020ba906 "xen/netback: Multiple tasklets support." changed
netbk_gop_frag_copy to attempt to lookup a pending_tx_info for any
foreign page, regardless of whether the page was a netback-foreign
page.

In the case of non-netback pages this can lead to dereferencing a NULL
src_pend->netif.

Restore the behaviour of netif_page_index prior toa3031942
"xen/netback: Introduce a new struct type page_ext" by performing
tests to ensure that page is a netback page and extend the same checks
to netif_page_group.

Actually combine netif_page_{index,group} in to a single function
since they are always called together and it saves duplicating all the
checks.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Cc: Xu, Dongxiao <dongxiao.xu@xxxxxxxxx>
---
 drivers/xen/netback/netback.c |   56 ++++++++++++++++++++++++++++------------
 1 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 0ed7e61..ed7cd65 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -89,18 +89,37 @@ static inline void netif_set_page_ext(struct page *pg, 
unsigned int group,
        pg->mapping = ext.mapping;
 }
 
-static inline unsigned int netif_page_group(const struct page *pg)
+static inline int netif_get_page_ext(struct page *pg, unsigned int *_group, 
unsigned int *_idx)
 {
        union page_ext ext = { .mapping = pg->mapping };
+       struct xen_netbk *netbk;
+       unsigned int group, idx;
 
-       return ext.e.group - 1;
-}
+       if (!PageForeign(pg))
+               return 0;
 
-static inline unsigned int netif_page_index(const struct page *pg)
-{
-       union page_ext ext = { .mapping = pg->mapping };
+       group = ext.e.group - 1;
+
+       if (group < 0 || group >= xen_netbk_group_nr)
+               return 0;
+
+       netbk = &xen_netbk[group];
+
+       if (netbk->mmap_pages == NULL)
+               return 0;
 
-       return ext.e.idx;
+       idx = ext.e.idx;
+
+       if ((idx < 0) || (idx >= MAX_PENDING_REQS))
+               return 0;
+
+       if (netbk->mmap_pages[idx] != pg)
+               return 0;
+
+       *_group = group;
+       *_idx = idx;
+
+       return 1;
 }
 
 /*
@@ -386,8 +405,12 @@ static void netbk_gop_frag_copy(struct xen_netif *netif,
 {
        struct gnttab_copy *copy_gop;
        struct netbk_rx_meta *meta;
-       int group = netif_page_group(page);
-       int idx = netif_page_index(page);
+       /*
+        * These variables a used iff netif_get_page_ext returns true,
+        * in which case they are guaranteed to be initialized.
+         */
+       unsigned int uninitialized_var(group), uninitialized_var(idx);
+       int foreign = netif_get_page_ext(page, &group, &idx);
        unsigned long bytes;
 
        /* Data must not cross a page boundary. */
@@ -445,7 +468,7 @@ static void netbk_gop_frag_copy(struct xen_netif *netif,
 
                copy_gop = npo->copy + npo->copy_prod++;
                copy_gop->flags = GNTCOPY_dest_gref;
-               if (PageForeign(page)) {
+               if (foreign) {
                        struct xen_netbk *netbk = &xen_netbk[group];
                        struct pending_tx_info *src_pend;
 
@@ -1546,14 +1569,13 @@ static void netif_idx_release(struct xen_netbk *netbk, 
u16 pending_idx)
 
 static void netif_page_release(struct page *page, unsigned int order)
 {
-       int group = netif_page_group(page);
-       int idx = netif_page_index(page);
-       struct xen_netbk *netbk = &xen_netbk[group];
+       unsigned int group, idx;
+       int foreign = netif_get_page_ext(page, &group, &idx);
+
+       BUG_ON(!foreign);
        BUG_ON(order);
-       BUG_ON(group < 0 || group >= xen_netbk_group_nr);
-       BUG_ON(idx < 0 || idx >= MAX_PENDING_REQS);
-       BUG_ON(netbk->mmap_pages[idx] != page);
-       netif_idx_release(netbk, idx);
+
+       netif_idx_release(&xen_netbk[group], idx);
 }
 
 irqreturn_t netif_be_int(int irq, void *dev_id)
-- 
1.5.6.5


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel