Re: Re: [PATCH] SUNRPC: Convert from atomic_t to refcount_t on rpc_clnt->cl_count

[Xiyu Yang]

Sorry, I'm not sure why you need to bump a zero refcount in a normal situation. But maybe we can use refcount_inc_not_zero() API in rpc_free_auth() instead?

> -----Original Messages-----
> From: "Trond Myklebust" <trondmy@xxxxxxxxxxxxxxx>
> Sent Time: 2021-07-17 22:43:26 (Saturday)
> To: "tanxin.ctf@xxxxxxxxx" <tanxin.ctf@xxxxxxxxx>, "xiyuyang19@xxxxxxxxxxxx" <xiyuyang19@xxxxxxxxxxxx>, "davem@xxxxxxxxxxxxx" <davem@xxxxxxxxxxxxx>, "chuck.lever@xxxxxxxxxx" <chuck.lever@xxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>, "kolga@xxxxxxxxxx" <kolga@xxxxxxxxxx>, "kuba@xxxxxxxxxx" <kuba@xxxxxxxxxx>, "netdev@xxxxxxxxxxxxxxx" <netdev@xxxxxxxxxxxxxxx>, "bfields@xxxxxxxxxxxx" <bfields@xxxxxxxxxxxx>, "anna.schumaker@xxxxxxxxxx" <anna.schumaker@xxxxxxxxxx>, "linux-nfs@xxxxxxxxxxxxxxx" <linux-nfs@xxxxxxxxxxxxxxx>
> Cc: "yuanxzhang@xxxxxxxxxxxx" <yuanxzhang@xxxxxxxxxxxx>
> Subject: Re: [PATCH] SUNRPC: Convert from atomic_t to refcount_t on rpc_clnt->cl_count
> 
> On Sat, 2021-07-17 at 18:18 +0800, Xiyu Yang wrote:
> > refcount_t type and corresponding API can protect refcounters from
> > accidental underflow and overflow and further use-after-free
> > situations.
> > 
> 
> Have you tested this patch? As far as I remember, the reason why we
> never converted is that refcount_inc() gets upset and WARNs when you
> bump a zero refcount, like we do very much on purpose in
> rpc_free_auth(). Is that no longer the case?
> 
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@xxxxxxxxxxxxxxx
> 
>