public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: stable@kernel.org, Paul Moore <paul.moore@hp.com>,
	David Woodhouse <David.Woodhouse@intel.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	qemu-devel <qemu-devel@nongnu.org>,
	Herbert Xu <herbert@gondor.hengli.com.au>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [stable] [PATCH] tun: orphan an skb on tx
Date: Wed, 21 Apr 2010 12:16:01 -0700	[thread overview]
Message-ID: <20100421191601.GJ13228@kroah.com> (raw)
In-Reply-To: <20100421113557.GA31606@redhat.com>

On Wed, Apr 21, 2010 at 02:35:57PM +0300, Michael S. Tsirkin wrote:
> On Tue, Apr 13, 2010 at 05:59:44PM +0300, Michael S. Tsirkin wrote:
> > The following situation was observed in the field:
> > tap1 sends packets, tap2 does not consume them, as a result
> > tap1 can not be closed. This happens because
> > tun/tap devices can hang on to skbs undefinitely.
> > 
> > As noted by Herbert, possible solutions include a timeout followed by a
> > copy/change of ownership of the skb, or always copying/changing
> > ownership if we're going into a hostile device.
> > 
> > This patch implements the second approach.
> > 
> > Note: one issue still remaining is that since skbs
> > keep reference to tun socket and tun socket has a
> > reference to tun device, we won't flush backlog,
> > instead simply waiting for all skbs to get transmitted.
> > At least this is not user-triggerable, and
> > this was not reported in practice, my assumption is
> > other devices besides tap complete an skb
> > within finite time after it has been queued.
> > 
> > A possible solution for the second issue
> > would not to have socket reference the device,
> > instead, implement dev->destructor for tun, and
> > wait for all skbs to complete there, but this
> > needs some thought, probably too risky for 2.6.34.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Tested-by: Yan Vugenfirer <yvugenfi@redhat.com>
> > 
> > ---
> > 
> > Please review the below, and consider for 2.6.34,
> > and stable trees.
> > 
> >  drivers/net/tun.c |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 96c39bd..4326520 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -387,6 +387,10 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
> >  		}
> >  	}
> >  
> > +	/* Orphan the skb - required as we might hang on to it
> > +	 * for indefinite time. */
> > +	skb_orphan(skb);
> > +
> >  	/* Enqueue packet */
> >  	skb_queue_tail(&tun->socket.sk->sk_receive_queue, skb);
> >  	dev->trans_start = jiffies;
> > -- 
> > 1.7.0.2.280.gc6f05
> 
> This is commit 0110d6f22f392f976e84ab49da1b42f85b64a3c5 in net-2.6
> Please cherry-pick this fix in stable kernels (2.6.32 and 2.6.33).

David Miller queues up the patches for the network subsystem for the
stable trees, and then forwards them to me when he feels they are ready.

So I'll defer to him on this one and wait for it to come from him.

thanks,

greg k-h

  parent reply	other threads:[~2010-04-21 19:22 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-13 14:59 [PATCH] tun: orphan an skb on tx Michael S. Tsirkin
2010-04-13 15:12 ` Herbert Xu
2010-04-13 15:36 ` Jan Kiszka
2010-04-13 16:40   ` Eric Dumazet
2010-04-13 16:52     ` Jan Kiszka
2010-04-13 17:39     ` Michael S. Tsirkin
2010-04-13 18:31       ` Eric Dumazet
2010-04-13 20:25         ` Michael S. Tsirkin
2010-04-13 20:38           ` Eric Dumazet
2010-04-13 20:43             ` Michael S. Tsirkin
2010-04-14  0:58         ` Herbert Xu
2010-04-14 11:55           ` David Miller
2015-02-01 11:20           ` David Woodhouse
2015-02-01 12:26             ` Michael S. Tsirkin
2015-02-01 13:33               ` David Woodhouse
2015-02-01 20:19                 ` David Miller
2015-02-01 21:29                   ` David Woodhouse
2015-02-02  5:07                     ` David Miller
2015-02-02  7:27                       ` David Woodhouse
2015-02-02  8:24                         ` Steffen Klassert
2015-02-02 15:30                           ` David Woodhouse
2015-02-02 15:23                         ` Phil Sutter
2015-02-02 15:47                           ` David Woodhouse
2015-02-04  0:19                         ` David Miller
2015-02-04  6:35                           ` David Woodhouse
2010-04-21 11:35 ` Michael S. Tsirkin
2010-04-21 11:46   ` Jan Kiszka
2010-04-21 11:45     ` Michael S. Tsirkin
2010-04-21 19:16   ` Greg KH [this message]
2010-09-14 15:20 ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100421191601.GJ13228@kroah.com \
    --to=greg@kroah.com \
    --cc=David.Woodhouse@intel.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.hengli.com.au \
    --cc=jan.kiszka@siemens.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=paul.moore@hp.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox