From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net-next] tuntap: fix possible deadlock when fail to register netdev Date: Fri, 8 Dec 2017 11:27:22 +0800 Message-ID: <8ad598ed-90dd-c2e4-02f4-7cc87bea3e8c@redhat.com> References: <1512701655-18751-1-git-send-email-jasowang@redhat.com> <1512702678.25033.20.camel@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: mst@redhat.com, Willem de Bruijn To: Eric Dumazet , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59874 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751904AbdLHD1c (ORCPT ); Thu, 7 Dec 2017 22:27:32 -0500 In-Reply-To: <1512702678.25033.20.camel@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2017年12月08日 11:11, Eric Dumazet wrote: > On Fri, 2017-12-08 at 10:54 +0800, Jason Wang wrote: >> Private destructor could be called when register_netdev() fail with >> rtnl lock held. This will lead deadlock in tun_free_netdev() who >> tries >> to hold rtnl_lock. Fixing this by switching to use spinlock to >> synchronize. >> >> Fixes: 96f84061620c ("tun: add eBPF based queue selection method") >> Reported-by: Eric Dumazet >> Cc: Eric Dumazet >> Cc: Willem de Bruijn >> Signed-off-by: Jason Wang >> --- >>  drivers/net/tun.c | 7 ++++--- >>  1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c >> index 787cc35..f7ccd79 100644 >> --- a/drivers/net/tun.c >> +++ b/drivers/net/tun.c >> @@ -2050,8 +2050,11 @@ static int __tun_set_steering_ebpf(struct >> tun_struct *tun, >>   new->prog = prog; >>   } >> >> - old = rtnl_dereference(tun->steering_prog); >> + spin_lock(&tun->lock); >> + old = rcu_dereference_protected(tun->steering_prog, >> + lock_is_held(&tun->lock)); >>   rcu_assign_pointer(tun->steering_prog, new); >> + spin_unlock(&tun->lock); >> > Hi Jason, thank you for the following up. > > Have you tested this code path with lockdep enabled ? No I test without it. > > My gut feeling is that you need spin_lock_bh() here. > > Thanks > Yes, I miss the fact this the lock is used by e.g flow caches too. Will post V2. Thanks