[[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Wei Liu]

With the introduction of "xen-netback: Don't destroy the netdev until
the vif is shut down" (upstream commit id 279f438e36), vif disconnect
and free are separated. However in the backported verion reference
counting code was not correctly modified, and the reset of vif->tx_irq
was lost. If frontend goes through vif life cycle more than once the
reference counting is skewed.

This patch adds back the missing tx_irq reset line. It also moves
several lines of the reference counting code to vif_free, so the moved
code corresponds to the counterpart in vif_alloc, thus the reference
counting is balanced.

3.12 and onward versions are not affected by this bug, because reference
counting code was removed due to the introduction of 1:1 model.

This pacth should be backported to all stable verions which are lower
than 3.12 and have 279f438e36.

Reported-and-tested-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netback/interface.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index d28324a..342d4e5 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -418,9 +418,6 @@ void xenvif_disconnect(struct xenvif *vif)
 	if (netif_carrier_ok(vif->dev))
 		xenvif_carrier_off(vif);
 
-	atomic_dec(&vif->refcnt);
-	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
-
 	if (vif->tx_irq) {
 		if (vif->tx_irq == vif->rx_irq)
 			unbind_from_irqhandler(vif->tx_irq, vif);
@@ -428,6 +425,7 @@ void xenvif_disconnect(struct xenvif *vif)
 			unbind_from_irqhandler(vif->tx_irq, vif);
 			unbind_from_irqhandler(vif->rx_irq, vif);
 		}
+		vif->tx_irq = 0;
 	}
 
 	xen_netbk_unmap_frontend_rings(vif);
@@ -435,6 +433,9 @@ void xenvif_disconnect(struct xenvif *vif)
 
 void xenvif_free(struct xenvif *vif)
 {
+	atomic_dec(&vif->refcnt);
+	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
+
 	unregister_netdev(vif->dev);
 
 	free_netdev(vif->dev);
-- 
1.7.10.4

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Ian Campbell]

On Thu, 2013-11-28 at 12:48 +0000, Wei Liu wrote:
> With the introduction of "xen-netback: Don't destroy the netdev until
> the vif is shut down" (upstream commit id 279f438e36), vif disconnect
> and free are separated. However in the backported verion reference
> counting code was not correctly modified, and the reset of vif->tx_irq
> was lost. If frontend goes through vif life cycle more than once the
> reference counting is skewed.
> 
> This patch adds back the missing tx_irq reset line. It also moves
> several lines of the reference counting code to vif_free, so the moved
> code corresponds to the counterpart in vif_alloc, thus the reference
> counting is balanced.

Is this a requirement of the bugfix or just to make reasoning about
things easier?

> 3.12 and onward versions are not affected by this bug, because reference
> counting code was removed due to the introduction of 1:1 model.
> 
> This pacth should be backported to all stable verions which are lower
> than 3.12 and have 279f438e36.
> 
> Reported-and-tested-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

> Cc: Konrad Wilk <konrad.wilk@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
>  drivers/net/xen-netback/interface.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index d28324a..342d4e5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -418,9 +418,6 @@ void xenvif_disconnect(struct xenvif *vif)
>  	if (netif_carrier_ok(vif->dev))
>  		xenvif_carrier_off(vif);
>  
> -	atomic_dec(&vif->refcnt);
> -	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
> -
>  	if (vif->tx_irq) {
>  		if (vif->tx_irq == vif->rx_irq)
>  			unbind_from_irqhandler(vif->tx_irq, vif);
> @@ -428,6 +425,7 @@ void xenvif_disconnect(struct xenvif *vif)
>  			unbind_from_irqhandler(vif->tx_irq, vif);
>  			unbind_from_irqhandler(vif->rx_irq, vif);
>  		}
> +		vif->tx_irq = 0;
>  	}
>  
>  	xen_netbk_unmap_frontend_rings(vif);
> @@ -435,6 +433,9 @@ void xenvif_disconnect(struct xenvif *vif)
>  
>  void xenvif_free(struct xenvif *vif)
>  {
> +	atomic_dec(&vif->refcnt);
> +	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
> +
>  	unregister_netdev(vif->dev);
>  
>  	free_netdev(vif->dev);

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Wei Liu]

On Thu, Nov 28, 2013 at 01:08:53PM +0000, Ian Campbell wrote:
> On Thu, 2013-11-28 at 12:48 +0000, Wei Liu wrote:
> > With the introduction of "xen-netback: Don't destroy the netdev until
> > the vif is shut down" (upstream commit id 279f438e36), vif disconnect
> > and free are separated. However in the backported verion reference
> > counting code was not correctly modified, and the reset of vif->tx_irq
> > was lost. If frontend goes through vif life cycle more than once the
> > reference counting is skewed.
> > 
> > This patch adds back the missing tx_irq reset line. It also moves
> > several lines of the reference counting code to vif_free, so the moved
> > code corresponds to the counterpart in vif_alloc, thus the reference
> > counting is balanced.
> 
> Is this a requirement of the bugfix or just to make reasoning about
> things easier?
> 

The latter.

Wei.

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[David Miller]

From: Wei Liu <wei.liu2@citrix.com>
Date: Thu, 28 Nov 2013 13:13:46 +0000

> On Thu, Nov 28, 2013 at 01:08:53PM +0000, Ian Campbell wrote:
>> On Thu, 2013-11-28 at 12:48 +0000, Wei Liu wrote:
>> > With the introduction of "xen-netback: Don't destroy the netdev until
>> > the vif is shut down" (upstream commit id 279f438e36), vif disconnect
>> > and free are separated. However in the backported verion reference
>> > counting code was not correctly modified, and the reset of vif->tx_irq
>> > was lost. If frontend goes through vif life cycle more than once the
>> > reference counting is skewed.
>> > 
>> > This patch adds back the missing tx_irq reset line. It also moves
>> > several lines of the reference counting code to vif_free, so the moved
>> > code corresponds to the counterpart in vif_alloc, thus the reference
>> > counting is balanced.
>> 
>> Is this a requirement of the bugfix or just to make reasoning about
>> things easier?
>> 
> 
> The latter.

I've queued this up for -stable, thanks.

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Greg KH]

On Thu, Nov 28, 2013 at 12:48:09PM +0000, Wei Liu wrote:
> With the introduction of "xen-netback: Don't destroy the netdev until
> the vif is shut down" (upstream commit id 279f438e36), vif disconnect
> and free are separated. However in the backported verion reference
> counting code was not correctly modified, and the reset of vif->tx_irq
> was lost. If frontend goes through vif life cycle more than once the
> reference counting is skewed.
> 
> This patch adds back the missing tx_irq reset line. It also moves
> several lines of the reference counting code to vif_free, so the moved
> code corresponds to the counterpart in vif_alloc, thus the reference
> counting is balanced.
> 
> 3.12 and onward versions are not affected by this bug, because reference
> counting code was removed due to the introduction of 1:1 model.
> 
> This pacth should be backported to all stable verions which are lower
> than 3.12 and have 279f438e36.

What stable versions are those?  279f438e36 showed up in 3.12-rc2, and
was not marked for stable inclusion, so what kernel would end up needing
this patch?

confused,

greg k-h

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Wei Liu]

On Sat, Nov 30, 2013 at 08:48:35PM -0800, Greg KH wrote:
> On Thu, Nov 28, 2013 at 12:48:09PM +0000, Wei Liu wrote:
> > With the introduction of "xen-netback: Don't destroy the netdev until
> > the vif is shut down" (upstream commit id 279f438e36), vif disconnect
> > and free are separated. However in the backported verion reference
> > counting code was not correctly modified, and the reset of vif->tx_irq
> > was lost. If frontend goes through vif life cycle more than once the
> > reference counting is skewed.
> > 
> > This patch adds back the missing tx_irq reset line. It also moves
> > several lines of the reference counting code to vif_free, so the moved
> > code corresponds to the counterpart in vif_alloc, thus the reference
> > counting is balanced.
> > 
> > 3.12 and onward versions are not affected by this bug, because reference
> > counting code was removed due to the introduction of 1:1 model.
> > 
> > This pacth should be backported to all stable verions which are lower
> > than 3.12 and have 279f438e36.
> 
> What stable versions are those?  279f438e36 showed up in 3.12-rc2, and
> was not marked for stable inclusion, so what kernel would end up needing
> this patch?
> 

Greg,

279f438e36 was backported to at least stable branches 

  3.11.y: <13826815042174@kroah.org> commit id f495ddc46
  3.10.y: <13828112161592@kroah.com> commit id a4626bf64

So at least those two need this patch.

And I remeber 3.8.y.z extended stable branch maintained by Kamal also
has the said commit.

> confused,
> 

Sorry, I should've stated cleared all versions.

BTW the above patch applies to 3.11.y. I can provide backports to 3.10.y
as well, if necessary.

Wei.

> greg k-h

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Greg KH]

On Mon, Dec 02, 2013 at 12:01:34PM +0000, Wei Liu wrote:
> On Sat, Nov 30, 2013 at 08:48:35PM -0800, Greg KH wrote:
> > On Thu, Nov 28, 2013 at 12:48:09PM +0000, Wei Liu wrote:
> > > With the introduction of "xen-netback: Don't destroy the netdev until
> > > the vif is shut down" (upstream commit id 279f438e36), vif disconnect
> > > and free are separated. However in the backported verion reference
> > > counting code was not correctly modified, and the reset of vif->tx_irq
> > > was lost. If frontend goes through vif life cycle more than once the
> > > reference counting is skewed.
> > > 
> > > This patch adds back the missing tx_irq reset line. It also moves
> > > several lines of the reference counting code to vif_free, so the moved
> > > code corresponds to the counterpart in vif_alloc, thus the reference
> > > counting is balanced.
> > > 
> > > 3.12 and onward versions are not affected by this bug, because reference
> > > counting code was removed due to the introduction of 1:1 model.
> > > 
> > > This pacth should be backported to all stable verions which are lower
> > > than 3.12 and have 279f438e36.
> > 
> > What stable versions are those?  279f438e36 showed up in 3.12-rc2, and
> > was not marked for stable inclusion, so what kernel would end up needing
> > this patch?
> > 
> 
> Greg,
> 
> 279f438e36 was backported to at least stable branches 
> 
>   3.11.y: <13826815042174@kroah.org> commit id f495ddc46
>   3.10.y: <13828112161592@kroah.com> commit id a4626bf64
> 
> So at least those two need this patch.
> 
> And I remeber 3.8.y.z extended stable branch maintained by Kamal also
> has the said commit.
> 
> > confused,
> > 
> 
> Sorry, I should've stated cleared all versions.
> 
> BTW the above patch applies to 3.11.y. I can provide backports to 3.10.y
> as well, if necessary.

As 3.11.y is now dead, there's not much I can do with that one, but I
could use a backport for 3.10.y, as this patch does not apply there,
thanks.

greg k-h

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Wei Liu]

On Mon, Dec 02, 2013 at 09:19:58AM -0800, Greg KH wrote:
[...]
> As 3.11.y is now dead, there's not much I can do with that one, but I
> could use a backport for 3.10.y, as this patch does not apply there,
> thanks.
> 

Here it is.

---8<---
>From f92adc90042cb725cf6351141f52e9fc71639a89 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 2 Dec 2013 17:44:09 +0000
Subject: [PATCH] xen-netback: fix refcnt unbalance for 3.10

With the introduction of "xen-netback: Don't destroy the netdev until
the vif is shut down" (upstream commit id 279f438e36), vif disconnect
and free are separated. However in the backported version reference
counting code was not correctly modified, and the reset of vif->irq
was lost. If frontend goes through vif life cycle more than once the
reference counting is skewed.

This patch adds back the missing vif->irq reset line. It also moves
several lines of the reference counting code to vif_free, so the moved
code corresponds to the counterpart in vif_alloc, thus the reference
counting is balanced.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
 drivers/net/xen-netback/interface.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index c4a2eb2..540a796 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -365,17 +365,19 @@ void xenvif_disconnect(struct xenvif *vif)
 	if (netif_carrier_ok(vif->dev))
 		xenvif_carrier_off(vif);
 
-	atomic_dec(&vif->refcnt);
-	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
-
-	if (vif->irq)
+	if (vif->irq) {
 		unbind_from_irqhandler(vif->irq, vif);
+		vif->irq = 0;
+	}
 
 	xen_netbk_unmap_frontend_rings(vif);
 }
 
 void xenvif_free(struct xenvif *vif)
 {
+	atomic_dec(&vif->refcnt);
+	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
+
 	unregister_netdev(vif->dev);
 
 	free_netdev(vif->dev);
-- 
1.7.10.4


> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Greg KH]

On Mon, Dec 02, 2013 at 05:49:54PM +0000, Wei Liu wrote:
> On Mon, Dec 02, 2013 at 09:19:58AM -0800, Greg KH wrote:
> [...]
> > As 3.11.y is now dead, there's not much I can do with that one, but I
> > could use a backport for 3.10.y, as this patch does not apply there,
> > thanks.
> > 
> 
> Here it is.

Thanks, that worked.

greg k-h

Patch "xen-netback: fix refcnt unbalance for 3.10" has been added to the 3.10-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    xen-netback: fix refcnt unbalance for 3.10

to the 3.10-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     xen-netback-fix-refcnt-unbalance-for-3.10.patch
and it can be found in the queue-3.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From wei.liu2@citrix.com  Mon Dec  2 09:57:28 2013
From: Wei Liu <wei.liu2@citrix.com>
Date: Mon, 2 Dec 2013 17:49:54 +0000
Subject: xen-netback: fix refcnt unbalance for 3.10
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Wei Liu <wei.liu2@citrix.com>, <xen-devel@lists.xen.org>, <netdev@vger.kernel.org>, <stable@vger.kernel.org>, <tomasz.wroblewski@citrix.com>, Ian Campbell <ian.campbell@citrix.com>, Konrad Wilk <konrad.wilk@oracle.com>, David Vrabel <david.vrabel@citrix.com>
Message-ID: <20131202174954.GD14293@zion.uk.xensource.com>
Content-Disposition: inline

From: Wei Liu <wei.liu2@citrix.com>

With the introduction of "xen-netback: Don't destroy the netdev until
the vif is shut down" (upstream commit id 279f438e36), vif disconnect
and free are separated. However in the backported version reference
counting code was not correctly modified, and the reset of vif->irq
was lost. If frontend goes through vif life cycle more than once the
reference counting is skewed.

This patch adds back the missing vif->irq reset line. It also moves
several lines of the reference counting code to vif_free, so the moved
code corresponds to the counterpart in vif_alloc, thus the reference
counting is balanced.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Konrad Wilk <konrad.wilk@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/xen-netback/interface.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -365,17 +365,19 @@ void xenvif_disconnect(struct xenvif *vi
 	if (netif_carrier_ok(vif->dev))
 		xenvif_carrier_off(vif);
 
-	atomic_dec(&vif->refcnt);
-	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
-
-	if (vif->irq)
+	if (vif->irq) {
 		unbind_from_irqhandler(vif->irq, vif);
+		vif->irq = 0;
+	}
 
 	xen_netbk_unmap_frontend_rings(vif);
 }
 
 void xenvif_free(struct xenvif *vif)
 {
+	atomic_dec(&vif->refcnt);
+	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
+
 	unregister_netdev(vif->dev);
 
 	free_netdev(vif->dev);


Patches currently in stable-queue which might be from wei.liu2@citrix.com are

queue-3.10/xen-netback-fix-refcnt-unbalance-for-3.10.patch

Re: [[PATCH stable <3.12]] xen-netback: fix refcnt unbalance for 3.11 and earlier versions

[Luis Henriques]

On Thu, Nov 28, 2013 at 12:48:09PM +0000, Wei Liu wrote:
> With the introduction of "xen-netback: Don't destroy the netdev until
> the vif is shut down" (upstream commit id 279f438e36), vif disconnect
> and free are separated. However in the backported verion reference
> counting code was not correctly modified, and the reset of vif->tx_irq
> was lost. If frontend goes through vif life cycle more than once the
> reference counting is skewed.
> 
> This patch adds back the missing tx_irq reset line. It also moves
> several lines of the reference counting code to vif_free, so the moved
> code corresponds to the counterpart in vif_alloc, thus the reference
> counting is balanced.
> 
> 3.12 and onward versions are not affected by this bug, because reference
> counting code was removed due to the introduction of 1:1 model.
> 
> This pacth should be backported to all stable verions which are lower
> than 3.12 and have 279f438e36.

For some reason, I missed this patch.  I'm queuing it now for the 3.11
kernel.  Thank you!

Cheers,
--
Luis

> 
> Reported-and-tested-by: Tomasz Wroblewski <tomasz.wroblewski@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Konrad Wilk <konrad.wilk@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
>  drivers/net/xen-netback/interface.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index d28324a..342d4e5 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -418,9 +418,6 @@ void xenvif_disconnect(struct xenvif *vif)
>  	if (netif_carrier_ok(vif->dev))
>  		xenvif_carrier_off(vif);
>  
> -	atomic_dec(&vif->refcnt);
> -	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
> -
>  	if (vif->tx_irq) {
>  		if (vif->tx_irq == vif->rx_irq)
>  			unbind_from_irqhandler(vif->tx_irq, vif);
> @@ -428,6 +425,7 @@ void xenvif_disconnect(struct xenvif *vif)
>  			unbind_from_irqhandler(vif->tx_irq, vif);
>  			unbind_from_irqhandler(vif->rx_irq, vif);
>  		}
> +		vif->tx_irq = 0;
>  	}
>  
>  	xen_netbk_unmap_frontend_rings(vif);
> @@ -435,6 +433,9 @@ void xenvif_disconnect(struct xenvif *vif)
>  
>  void xenvif_free(struct xenvif *vif)
>  {
> +	atomic_dec(&vif->refcnt);
> +	wait_event(vif->waiting_to_free, atomic_read(&vif->refcnt) == 0);
> +
>  	unregister_netdev(vif->dev);
>  
>  	free_netdev(vif->dev);
> -- 
> 1.7.10.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe stable" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html