netdev - Re: [stable] [PATCH] tun: orphan an skb on tx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 21 Apr 2010 12:16:01 -0700
From:	Greg KH <greg@...ah.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	stable@...nel.org, Paul Moore <paul.moore@...com>,
	David Woodhouse <David.Woodhouse@...el.com>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
	qemu-devel <qemu-devel@...gnu.org>,
	Herbert Xu <herbert@...dor.hengli.com.au>,
	Jan Kiszka <jan.kiszka@...mens.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [stable] [PATCH] tun: orphan an skb on tx

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@...hat.com>
> > Tested-by: Yan Vugenfirer <yvugenfi@...hat.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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html