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

[Trond Myklebust <trondmy-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>]

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@intel.com>
> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
>  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@primarydata.com