[PATCH v4 net] xen-netback: disable rogue vif in kthread context

[PATCH v4 net] xen-netback: disable rogue vif in kthread context

[Wei Liu]

When netback discovers frontend is sending malformed packet it will
disables the interface which serves that frontend.

However disabling a network interface involving taking a mutex which
cannot be done in softirq context, so we need to defer this process to
kthread context.

This patch does the following:
1. introduce a flag to indicate the interface is disabled.
2. check that flag in TX path, don't do any work if it's true.
3. check that flag in RX path, turn off that interface if it's true.

The reason to disable it in RX path is because RX uses kthread. After
this change the behavior of netback is still consistent -- it won't do
any TX work for a rogue frontend, and the interface will be eventually
turned off.

Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
doesn't make sense to continue processing packets if frontend is rogue.

This is a fix for XSA-90.

Reported-by: Török Edwin <edwin@etorok.net>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
v4: add vif->disabled to kthread wait_event_interruptible
v3: use napi_complete as it does all the necessary work for us
---
 drivers/net/xen-netback/common.h    |    5 +++++
 drivers/net/xen-netback/interface.c |   11 +++++++++++
 drivers/net/xen-netback/netback.c   |   16 ++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index ae413a2..4bf5b33 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -113,6 +113,11 @@ struct xenvif {
 	domid_t          domid;
 	unsigned int     handle;
 
+	/* Is this interface disabled? True when backend discovers
+	 * frontend is rogue.
+	 */
+	bool disabled;
+
 	/* Use NAPI for guest TX */
 	struct napi_struct napi;
 	/* When feature-split-event-channels = 0, tx_irq = rx_irq. */
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 301cc03..2e92d52 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -62,6 +62,15 @@ static int xenvif_poll(struct napi_struct *napi, int budget)
 	struct xenvif *vif = container_of(napi, struct xenvif, napi);
 	int work_done;
 
+	/* This vif is rogue, we pretend we've there is nothing to do
+	 * for this vif to deschedule it from NAPI. But this interface
+	 * will be turned off in thread context later.
+	 */
+	if (unlikely(vif->disabled)) {
+		napi_complete(napi);
+		return 0;
+	}
+
 	work_done = xenvif_tx_action(vif, budget);
 
 	if (work_done < budget) {
@@ -321,6 +330,8 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 	vif->ip_csum = 1;
 	vif->dev = dev;
 
+	vif->disabled = false;
+
 	vif->credit_bytes = vif->remaining_credit = ~0UL;
 	vif->credit_usec  = 0UL;
 	init_timer(&vif->credit_timeout);
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index cd0bd95..97030c1 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -680,7 +680,8 @@ static void xenvif_tx_err(struct xenvif *vif,
 static void xenvif_fatal_tx_err(struct xenvif *vif)
 {
 	netdev_err(vif->dev, "fatal error; disabling device\n");
-	xenvif_carrier_off(vif);
+	vif->disabled = true;
+	xenvif_kick_thread(vif);
 }
 
 static int xenvif_count_requests(struct xenvif *vif,
@@ -1151,7 +1152,7 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
 				   vif->tx.sring->req_prod, vif->tx.req_cons,
 				   XEN_NETIF_TX_RING_SIZE);
 			xenvif_fatal_tx_err(vif);
-			continue;
+			break;
 		}
 
 		work_to_do = RING_HAS_UNCONSUMED_REQUESTS(&vif->tx);
@@ -1573,7 +1574,18 @@ int xenvif_kthread(void *data)
 	while (!kthread_should_stop()) {
 		wait_event_interruptible(vif->wq,
 					 rx_work_todo(vif) ||
+					 vif->disabled ||
 					 kthread_should_stop());
+
+		/* This frontend is found to be rogue, disable it in
+		 * kthread context. Currently this is only set when
+		 * netback finds out frontend sends malformed packet,
+		 * but we cannot disable the interface in softirq
+		 * context so we defer it here.
+		 */
+		if (unlikely(vif->disabled && netif_carrier_ok(vif->dev)))
+			xenvif_carrier_off(vif);
+
 		if (kthread_should_stop())
 			break;
 
-- 
1.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[Wei Liu]

On Wed, Apr 02, 2014 at 10:05:40AM +0100, Ian Campbell wrote:
> On Tue, 2014-04-01 at 16:27 -0400, David Miller wrote:
> > From: Wei Liu <wei.liu2@citrix.com>
> > Date: Tue, 1 Apr 2014 12:46:12 +0100
> > 
> > > When netback discovers frontend is sending malformed packet it will
> > > disables the interface which serves that frontend.
> > > 
> > > However disabling a network interface involving taking a mutex which
> > > cannot be done in softirq context, so we need to defer this process to
> > > kthread context.
> > > 
> > > This patch does the following:
> > > 1. introduce a flag to indicate the interface is disabled.
> > > 2. check that flag in TX path, don't do any work if it's true.
> > > 3. check that flag in RX path, turn off that interface if it's true.
> > > 
> > > The reason to disable it in RX path is because RX uses kthread. After
> > > this change the behavior of netback is still consistent -- it won't do
> > > any TX work for a rogue frontend, and the interface will be eventually
> > > turned off.
> > > 
> > > Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> > > doesn't make sense to continue processing packets if frontend is rogue.
> > > 
> > > This is a fix for XSA-90.
> > > 
> > > Reported-by: Török Edwin <edwin@etorok.net>
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Applied.
> 
> Thanks.
> 
> Wei -- can you advise as to which stable trees this should be queued
> for? Looks like 3.12 onwards according to
> http://xenbits.xen.org/xsa/advisory-90.html which says the issue arose
> between 3.11 and 3.12-rc1?
> 

Correct.

Wei.

> Cheers,
> Ian.
> 

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[Wei Liu]

On Wed, Apr 02, 2014 at 10:05:40AM +0100, Ian Campbell wrote:
> On Tue, 2014-04-01 at 16:27 -0400, David Miller wrote:
> > From: Wei Liu <wei.liu2@citrix.com>
> > Date: Tue, 1 Apr 2014 12:46:12 +0100
> > 
> > > When netback discovers frontend is sending malformed packet it will
> > > disables the interface which serves that frontend.
> > > 
> > > However disabling a network interface involving taking a mutex which
> > > cannot be done in softirq context, so we need to defer this process to
> > > kthread context.
> > > 
> > > This patch does the following:
> > > 1. introduce a flag to indicate the interface is disabled.
> > > 2. check that flag in TX path, don't do any work if it's true.
> > > 3. check that flag in RX path, turn off that interface if it's true.
> > > 
> > > The reason to disable it in RX path is because RX uses kthread. After
> > > this change the behavior of netback is still consistent -- it won't do
> > > any TX work for a rogue frontend, and the interface will be eventually
> > > turned off.
> > > 
> > > Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> > > doesn't make sense to continue processing packets if frontend is rogue.
> > > 
> > > This is a fix for XSA-90.
> > > 
> > > Reported-by: Török Edwin <edwin@etorok.net>
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > Applied.
> 
> Thanks.
> 
> Wei -- can you advise as to which stable trees this should be queued
> for? Looks like 3.12 onwards according to
> http://xenbits.xen.org/xsa/advisory-90.html which says the issue arose
> between 3.11 and 3.12-rc1?
> 

Correct.

Wei.

> Cheers,
> Ian.
> 

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[Ian Campbell]

On Tue, 2014-04-01 at 16:27 -0400, David Miller wrote:
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Tue, 1 Apr 2014 12:46:12 +0100
> 
> > When netback discovers frontend is sending malformed packet it will
> > disables the interface which serves that frontend.
> > 
> > However disabling a network interface involving taking a mutex which
> > cannot be done in softirq context, so we need to defer this process to
> > kthread context.
> > 
> > This patch does the following:
> > 1. introduce a flag to indicate the interface is disabled.
> > 2. check that flag in TX path, don't do any work if it's true.
> > 3. check that flag in RX path, turn off that interface if it's true.
> > 
> > The reason to disable it in RX path is because RX uses kthread. After
> > this change the behavior of netback is still consistent -- it won't do
> > any TX work for a rogue frontend, and the interface will be eventually
> > turned off.
> > 
> > Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> > doesn't make sense to continue processing packets if frontend is rogue.
> > 
> > This is a fix for XSA-90.
> > 
> > Reported-by: Török Edwin <edwin@etorok.net>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> Applied.

Thanks.

Wei -- can you advise as to which stable trees this should be queued
for? Looks like 3.12 onwards according to
http://xenbits.xen.org/xsa/advisory-90.html which says the issue arose
between 3.11 and 3.12-rc1?

Cheers,
Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[David Miller]

From: Wei Liu <wei.liu2@citrix.com>
Date: Tue, 1 Apr 2014 12:46:12 +0100

> When netback discovers frontend is sending malformed packet it will
> disables the interface which serves that frontend.
> 
> However disabling a network interface involving taking a mutex which
> cannot be done in softirq context, so we need to defer this process to
> kthread context.
> 
> This patch does the following:
> 1. introduce a flag to indicate the interface is disabled.
> 2. check that flag in TX path, don't do any work if it's true.
> 3. check that flag in RX path, turn off that interface if it's true.
> 
> The reason to disable it in RX path is because RX uses kthread. After
> this change the behavior of netback is still consistent -- it won't do
> any TX work for a rogue frontend, and the interface will be eventually
> turned off.
> 
> Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> doesn't make sense to continue processing packets if frontend is rogue.
> 
> This is a fix for XSA-90.
> 
> Reported-by: Török Edwin <edwin@etorok.net>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Applied.

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[David Miller]

From: Wei Liu <wei.liu2@citrix.com>
Date: Tue, 1 Apr 2014 12:46:12 +0100

> When netback discovers frontend is sending malformed packet it will
> disables the interface which serves that frontend.
> 
> However disabling a network interface involving taking a mutex which
> cannot be done in softirq context, so we need to defer this process to
> kthread context.
> 
> This patch does the following:
> 1. introduce a flag to indicate the interface is disabled.
> 2. check that flag in TX path, don't do any work if it's true.
> 3. check that flag in RX path, turn off that interface if it's true.
> 
> The reason to disable it in RX path is because RX uses kthread. After
> this change the behavior of netback is still consistent -- it won't do
> any TX work for a rogue frontend, and the interface will be eventually
> turned off.
> 
> Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> doesn't make sense to continue processing packets if frontend is rogue.
> 
> This is a fix for XSA-90.
> 
> Reported-by: Török Edwin <edwin@etorok.net>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

Applied.

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[Ian Campbell]

On Tue, 2014-04-01 at 13:10 +0100, David Vrabel wrote:
> On 01/04/14 12:46, Wei Liu wrote:
> > When netback discovers frontend is sending malformed packet it will
> > disables the interface which serves that frontend.
> > 
> > However disabling a network interface involving taking a mutex which
> > cannot be done in softirq context, so we need to defer this process to
> > kthread context.
> > 
> > This patch does the following:
> > 1. introduce a flag to indicate the interface is disabled.
> > 2. check that flag in TX path, don't do any work if it's true.
> > 3. check that flag in RX path, turn off that interface if it's true.
> > 
> > The reason to disable it in RX path is because RX uses kthread. After
> > this change the behavior of netback is still consistent -- it won't do
> > any TX work for a rogue frontend, and the interface will be eventually
> > turned off.
> > 
> > Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> > doesn't make sense to continue processing packets if frontend is rogue.
> > 
> > This is a fix for XSA-90.
> 
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[Ian Campbell]

On Tue, 2014-04-01 at 13:10 +0100, David Vrabel wrote:
> On 01/04/14 12:46, Wei Liu wrote:
> > When netback discovers frontend is sending malformed packet it will
> > disables the interface which serves that frontend.
> > 
> > However disabling a network interface involving taking a mutex which
> > cannot be done in softirq context, so we need to defer this process to
> > kthread context.
> > 
> > This patch does the following:
> > 1. introduce a flag to indicate the interface is disabled.
> > 2. check that flag in TX path, don't do any work if it's true.
> > 3. check that flag in RX path, turn off that interface if it's true.
> > 
> > The reason to disable it in RX path is because RX uses kthread. After
> > this change the behavior of netback is still consistent -- it won't do
> > any TX work for a rogue frontend, and the interface will be eventually
> > turned off.
> > 
> > Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> > doesn't make sense to continue processing packets if frontend is rogue.
> > 
> > This is a fix for XSA-90.
> 
> Reviewed-by: David Vrabel <david.vrabel@citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[David Vrabel]

On 01/04/14 12:46, Wei Liu wrote:
> When netback discovers frontend is sending malformed packet it will
> disables the interface which serves that frontend.
> 
> However disabling a network interface involving taking a mutex which
> cannot be done in softirq context, so we need to defer this process to
> kthread context.
> 
> This patch does the following:
> 1. introduce a flag to indicate the interface is disabled.
> 2. check that flag in TX path, don't do any work if it's true.
> 3. check that flag in RX path, turn off that interface if it's true.
> 
> The reason to disable it in RX path is because RX uses kthread. After
> this change the behavior of netback is still consistent -- it won't do
> any TX work for a rogue frontend, and the interface will be eventually
> turned off.
> 
> Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> doesn't make sense to continue processing packets if frontend is rogue.
> 
> This is a fix for XSA-90.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

Re: [PATCH v4 net] xen-netback: disable rogue vif in kthread context

[David Vrabel]

On 01/04/14 12:46, Wei Liu wrote:
> When netback discovers frontend is sending malformed packet it will
> disables the interface which serves that frontend.
> 
> However disabling a network interface involving taking a mutex which
> cannot be done in softirq context, so we need to defer this process to
> kthread context.
> 
> This patch does the following:
> 1. introduce a flag to indicate the interface is disabled.
> 2. check that flag in TX path, don't do any work if it's true.
> 3. check that flag in RX path, turn off that interface if it's true.
> 
> The reason to disable it in RX path is because RX uses kthread. After
> this change the behavior of netback is still consistent -- it won't do
> any TX work for a rogue frontend, and the interface will be eventually
> turned off.
> 
> Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
> doesn't make sense to continue processing packets if frontend is rogue.
> 
> This is a fix for XSA-90.

Reviewed-by: David Vrabel <david.vrabel@citrix.com>

David

[PATCH v4 net] xen-netback: disable rogue vif in kthread context

[Wei Liu]

When netback discovers frontend is sending malformed packet it will
disables the interface which serves that frontend.

However disabling a network interface involving taking a mutex which
cannot be done in softirq context, so we need to defer this process to
kthread context.

This patch does the following:
1. introduce a flag to indicate the interface is disabled.
2. check that flag in TX path, don't do any work if it's true.
3. check that flag in RX path, turn off that interface if it's true.

The reason to disable it in RX path is because RX uses kthread. After
this change the behavior of netback is still consistent -- it won't do
any TX work for a rogue frontend, and the interface will be eventually
turned off.

Also change a "continue" to "break" after xenvif_fatal_tx_err, as it
doesn't make sense to continue processing packets if frontend is rogue.

This is a fix for XSA-90.

Reported-by: Török Edwin <edwin@etorok.net>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
v4: add vif->disabled to kthread wait_event_interruptible
v3: use napi_complete as it does all the necessary work for us
---
 drivers/net/xen-netback/common.h    |    5 +++++
 drivers/net/xen-netback/interface.c |   11 +++++++++++
 drivers/net/xen-netback/netback.c   |   16 ++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index ae413a2..4bf5b33 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -113,6 +113,11 @@ struct xenvif {
 	domid_t          domid;
 	unsigned int     handle;
 
+	/* Is this interface disabled? True when backend discovers
+	 * frontend is rogue.
+	 */
+	bool disabled;
+
 	/* Use NAPI for guest TX */
 	struct napi_struct napi;
 	/* When feature-split-event-channels = 0, tx_irq = rx_irq. */
diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 301cc03..2e92d52 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -62,6 +62,15 @@ static int xenvif_poll(struct napi_struct *napi, int budget)
 	struct xenvif *vif = container_of(napi, struct xenvif, napi);
 	int work_done;
 
+	/* This vif is rogue, we pretend we've there is nothing to do
+	 * for this vif to deschedule it from NAPI. But this interface
+	 * will be turned off in thread context later.
+	 */
+	if (unlikely(vif->disabled)) {
+		napi_complete(napi);
+		return 0;
+	}
+
 	work_done = xenvif_tx_action(vif, budget);
 
 	if (work_done < budget) {
@@ -321,6 +330,8 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid,
 	vif->ip_csum = 1;
 	vif->dev = dev;
 
+	vif->disabled = false;
+
 	vif->credit_bytes = vif->remaining_credit = ~0UL;
 	vif->credit_usec  = 0UL;
 	init_timer(&vif->credit_timeout);
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index cd0bd95..97030c1 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -680,7 +680,8 @@ static void xenvif_tx_err(struct xenvif *vif,
 static void xenvif_fatal_tx_err(struct xenvif *vif)
 {
 	netdev_err(vif->dev, "fatal error; disabling device\n");
-	xenvif_carrier_off(vif);
+	vif->disabled = true;
+	xenvif_kick_thread(vif);
 }
 
 static int xenvif_count_requests(struct xenvif *vif,
@@ -1151,7 +1152,7 @@ static unsigned xenvif_tx_build_gops(struct xenvif *vif, int budget)
 				   vif->tx.sring->req_prod, vif->tx.req_cons,
 				   XEN_NETIF_TX_RING_SIZE);
 			xenvif_fatal_tx_err(vif);
-			continue;
+			break;
 		}
 
 		work_to_do = RING_HAS_UNCONSUMED_REQUESTS(&vif->tx);
@@ -1573,7 +1574,18 @@ int xenvif_kthread(void *data)
 	while (!kthread_should_stop()) {
 		wait_event_interruptible(vif->wq,
 					 rx_work_todo(vif) ||
+					 vif->disabled ||
 					 kthread_should_stop());
+
+		/* This frontend is found to be rogue, disable it in
+		 * kthread context. Currently this is only set when
+		 * netback finds out frontend sends malformed packet,
+		 * but we cannot disable the interface in softirq
+		 * context so we defer it here.
+		 */
+		if (unlikely(vif->disabled && netif_carrier_ok(vif->dev)))
+			xenvif_carrier_off(vif);
+
 		if (kthread_should_stop())
 			break;
 
-- 
1.7.10.4