From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932312AbcERMlS (ORCPT ); Wed, 18 May 2016 08:41:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34259 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753031AbcERMlP (ORCPT ); Wed, 18 May 2016 08:41:15 -0400 Date: Wed, 18 May 2016 15:41:06 +0300 From: "Michael S. Tsirkin" To: Jason Wang Cc: davem@davemloft.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Eric Dumazet , Xi Wang Subject: Re: [PATCH net] tuntap: correctly wake up process during uninit Message-ID: <20160518140003-mutt-send-email-mst@redhat.com> References: <1463569097-3528-1-git-send-email-jasowang@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1463569097-3528-1-git-send-email-jasowang@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 18 May 2016 12:41:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, May 18, 2016 at 06:58:17PM +0800, Jason Wang wrote: > We used to check dev->reg_state against NETREG_REGISTERED after each > time we are woke up. But after commit 9e641bdcfa4e ("net-tun: > restructure tun_do_read for better sleep/wakeup efficiency"), it uses > skb_recv_datagram() which does not check dev->reg_state. This will > result if we delete a tun/tap device after a process is blocked in the > reading. The device will wait for the reference count which was held > by that process for ever. > > Fixes this by using RCV_SHUTDOWN which will be checked during > sk_recv_datagram() before trying to wake up the process during uninit. > > Fixes: 9e641bdcfa4e ("net-tun: restructure tun_do_read for better > sleep/wakeup efficiency") > > Cc: Eric Dumazet > Cc: Xi Wang > Cc: Michael S. Tsirkin > Signed-off-by: Jason Wang Acked-by: Michael S. Tsirkin > --- > The patch is needed for -stable. > --- > drivers/net/tun.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index 425e983..752d849 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -580,11 +580,13 @@ static void tun_detach_all(struct net_device *dev) > for (i = 0; i < n; i++) { > tfile = rtnl_dereference(tun->tfiles[i]); > BUG_ON(!tfile); > + tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN; > tfile->socket.sk->sk_data_ready(tfile->socket.sk); > RCU_INIT_POINTER(tfile->tun, NULL); > --tun->numqueues; > } > list_for_each_entry(tfile, &tun->disabled, next) { > + tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN; > tfile->socket.sk->sk_data_ready(tfile->socket.sk); > RCU_INIT_POINTER(tfile->tun, NULL); > } > @@ -641,6 +643,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte > goto out; > } > tfile->queue_index = tun->numqueues; > + tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN; > rcu_assign_pointer(tfile->tun, tun); > rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile); > tun->numqueues++; By the way I wonder: at the moment interface goes down each time userspace disconnects, even if it was persistent and brought up manually (as opposed to on file open). Should we maybe track manual link up status and keep persistent device up on userspace disconnect? > -- > 2.7.4