xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [GIT/PATCH 0/2] pair of netback fixes
@ 2010-07-30 14:16 Ian Campbell
  2010-07-30 14:16 ` [PATCH] xen: netback: do not unleash netback threads until initialisation is complete Ian Campbell
  2010-07-30 14:16 ` [PATCH] xen: netback: check if foreign pages are actually netback-created foreign pages Ian Campbell
  0 siblings, 2 replies; 3+ messages in thread
From: Ian Campbell @ 2010-07-30 14:16 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Dongxiao, Paul Durrant, xen-devel

I've had these sitting around for a while and only just got round to
posting them.

The second fixes a crash which I've only observed on the pvops netback
backported to the XCP kernel but which I believe is valid for upstream
too.

Ian.

The following changes since commit 37089b13602b0ccf44616324fa3a8abd77b610b4:
  Bastian Blank (1):
        xen/netback: Fix null-pointer access in netback_uevent

are available in the git repository at:

  git://xenbits.xensource.com/people/ianc/linux-2.6.git for-jeremy/netback

Ian Campbell (2):
      xen: netback: do not unleash netback threads until initialisation is complete
      xen: netback: check if foreign pages are actually netback-created foreign pages.

 drivers/xen/netback/netback.c |   60 ++++++++++++++++++++++++++++------------
 1 files changed, 42 insertions(+), 18 deletions(-)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH] xen: netback: do not unleash netback threads until initialisation is complete
  2010-07-30 14:16 [GIT/PATCH 0/2] pair of netback fixes Ian Campbell
@ 2010-07-30 14:16 ` Ian Campbell
  2010-07-30 14:16 ` [PATCH] xen: netback: check if foreign pages are actually netback-created foreign pages Ian Campbell
  1 sibling, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2010-07-30 14:16 UTC (permalink / raw)
  Cc: Jeremy Fitzhardinge, xen-devel, Paul Durrant, Ian Campbell,
	Xu, Dongxiao

Otherwise netbk_action_thread can reference &netbk->net_schedule_list
(via tx_work_todo) before it is initialised. Until now it was zeroed
which is probably safe but not exactly robust.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Xu, Dongxiao <dongxiao.xu@intel.com>
Cc: Paul Durrant <Paul.Durrant@citrix.com>
---
 drivers/xen/netback/netback.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 4121062..0ed7e61 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -1781,7 +1781,6 @@ static int __init netback_init(void)
 
 			if (!IS_ERR(netbk->kthread.task)) {
 				kthread_bind(netbk->kthread.task, group);
-				wake_up_process(netbk->kthread.task);
 			} else {
 				printk(KERN_ALERT
 					"kthread_run() fails at netback\n");
@@ -1807,6 +1806,9 @@ static int __init netback_init(void)
 		spin_lock_init(&netbk->net_schedule_list_lock);
 
 		atomic_set(&netbk->netfront_count, 0);
+
+		if (MODPARM_netback_kthread)
+			wake_up_process(netbk->kthread.task);
 	}
 
 	netbk_copy_skb_mode = NETBK_DONT_COPY_SKB;
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH] xen: netback: check if foreign pages are actually netback-created foreign pages.
  2010-07-30 14:16 [GIT/PATCH 0/2] pair of netback fixes Ian Campbell
  2010-07-30 14:16 ` [PATCH] xen: netback: do not unleash netback threads until initialisation is complete Ian Campbell
@ 2010-07-30 14:16 ` Ian Campbell
  1 sibling, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2010-07-30 14:16 UTC (permalink / raw)
  Cc: Jeremy Fitzhardinge, xen-devel, Ian Campbell, Xu, Dongxiao

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@citrix.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Xu, Dongxiao <dongxiao.xu@intel.com>
---
 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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-07-30 14:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-30 14:16 [GIT/PATCH 0/2] pair of netback fixes Ian Campbell
2010-07-30 14:16 ` [PATCH] xen: netback: do not unleash netback threads until initialisation is complete Ian Campbell
2010-07-30 14:16 ` [PATCH] xen: netback: check if foreign pages are actually netback-created foreign pages Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).