Re: [PATCH 01/23] net, sunrpc: convert rpc_cred.cr_count from atomic_t to refcount_t

[Trond Myklebust]

On Fri, 2017-03-17 at 14:10 +0200, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@xxxxxxxxx>
> Signed-off-by: Hans Liljestrand <ishkamiel@xxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Signed-off-by: David Windsor <dwindsor@xxxxxxxxx>
> ---
> Âinclude/linux/sunrpc/auth.h |ÂÂ8 ++++----
> Ânet/sunrpc/auth.cÂÂÂÂÂÂÂÂÂÂÂ| 12 ++++++------
> Â2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/sunrpc/auth.h
> b/include/linux/sunrpc/auth.h
> index b1bc62b..bd36e0b 100644
> --- a/include/linux/sunrpc/auth.h
> +++ b/include/linux/sunrpc/auth.h
> @@ -15,7 +15,7 @@
> Â#include <linux/sunrpc/msg_prot.h>
> Â#include <linux/sunrpc/xdr.h>
> Â
> -#include <linux/atomic.h>
> +#include <linux/refcount.h>
> Â#include <linux/rcupdate.h>
> Â#include <linux/uidgid.h>
> Â#include <linux/utsname.h>
> @@ -68,7 +68,7 @@ struct rpc_cred {
> Â#endif
> Â	unsigned long		cr_expire;	/* when to gc
> */
> Â	unsigned long		cr_flags;	/* various
> flags */
> -	atomic_t		cr_count;	/* ref count */
> +	refcount_t		cr_count;	/* ref count */
> 

NACK. That's going to be hitting WARN_ONCE(!refcount_inc_not_zero(r),
"refcount_t: increment on 0; use-after-free.\n") like there's no
tomorrow...

Please stop with these automated conversions. They are going to cause a
lot more bugs than they fix.

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@xxxxxxxxxxxxxxx