Re: [PATCH net 2/2] auth_gss: Fix deadlock that blocks rpcsec_gss_exit_net when use-gss-proxy==1

From: Trond Myklebust <trondmy@hammerspace.com>
To: "neilb@suse.com" <neilb@suse.com>,
	"timo@rothenpieler.org" <timo@rothenpieler.org>,
	"bfields@fieldses.org" <bfields@fieldses.org>,
	"tyhicks@canonical.com" <tyhicks@canonical.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"wanghai38@huawei.com" <wanghai38@huawei.com>,
	"nicolas.dichtel@6wind.com" <nicolas.dichtel@6wind.com>,
	"edumazet@google.com" <edumazet@google.com>,
	"jlayton@kernel.org" <jlayton@kernel.org>,
	"dsahern@gmail.com" <dsahern@gmail.com>,
	"christian.brauner@ubuntu.com" <christian.brauner@ubuntu.com>,
	"jiang.wang@bytedance.com" <jiang.wang@bytedance.com>,
	"anna.schumaker@netapp.com" <anna.schumaker@netapp.com>,
	"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"cong.wang@bytedance.com" <cong.wang@bytedance.com>,
	"ast@kernel.org" <ast@kernel.org>,
	"kuniyu@amazon.co.jp" <kuniyu@amazon.co.jp>,
	"willy@infradead.org" <willy@infradead.org>,
	"wenbin.zeng@gmail.com" <wenbin.zeng@gmail.com>,
	"tom@talpey.com" <tom@talpey.com>,
	"chuck.lever@oracle.com" <chuck.lever@oracle.com>,
	"Rao.Shoaib@oracle.com" <Rao.Shoaib@oracle.com>,
	"jakub.kicinski@netronome.com" <jakub.kicinski@netronome.com>,
	"kolga@netapp.com" <kolga@netapp.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net 2/2] auth_gss: Fix deadlock that blocks rpcsec_gss_exit_net when use-gss-proxy==1
Date: Tue, 28 Sep 2021 13:30:17 +0000	[thread overview]
Message-ID: <a845b544c6592e58feeaff3be9271a717f53b383.camel@hammerspace.com> (raw)
In-Reply-To: <20210928031440.2222303-3-wanghai38@huawei.com>

On Tue, 2021-09-28 at 11:14 +0800, Wang Hai wrote:
> When use-gss-proxy is set to 1, write_gssp() creates a rpc client in
> gssp_rpc_create(), this increases the netns refcount by 2, these
> refcounts are supposed to be released in rpcsec_gss_exit_net(), but
> it
> will never happen because rpcsec_gss_exit_net() is triggered only
> when
> the netns refcount gets to 0, specifically:
>     refcount=0 -> cleanup_net() -> ops_exit_list ->
> rpcsec_gss_exit_net
> It is a deadlock situation here, refcount will never get to 0 unless
> rpcsec_gss_exit_net() is called. So, in this case, the netns refcount
> should not be increased.
> 
> In this case, xprt will take a netns refcount which is not supposed
> to be taken. Add a new flag to rpc_create_args called
> RPC_CLNT_CREATE_NO_NET_REF for not increasing the netns refcount.
> 
> It is safe not to hold the netns refcount, because when
> cleanup_net(), it
> will hold the gssp_lock and then shut down the rpc client
> synchronously.
> 
> 
I don't like this solution at all. Adding this kind of flag is going to
lead to problems down the road.

Is there any reason whatsoever why we need this RPC client to exist
when there is no active knfsd server? IOW: Is there any reason why we
shouldn't defer creating this RPC client for when knfsd starts up in
this net namespace, and why we can't shut it down when knfsd shuts
down?

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com