Re: [PATCH] net: print proper warning on dst underflow

[Kees Cook]

On Tue, Sep 24, 2019 at 11:09:37AM +0200, Jason A. Donenfeld wrote:
> Proper warnings with stack traces make it much easier to figure out
> what's doing the double free and create more meaningful bug reports from
> users.
> 
> Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
> ---
>  net/core/dst.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/dst.c b/net/core/dst.c
> index 1325316d9eab..193af526e908 100644
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -172,7 +172,7 @@ void dst_release(struct dst_entry *dst)
>  		int newrefcnt;
>  
>  		newrefcnt = atomic_dec_return(&dst->__refcnt);
> -		if (unlikely(newrefcnt < 0))
> +		if (WARN_ONCE(newrefcnt < 0, "dst_release underflow"))
>  			net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
>  					     __func__, dst, newrefcnt);

Should __refcnt be a refcount_t to gain saturation protection? It seems
like going negative is bad...

-Kees

>  		if (!newrefcnt)
> @@ -187,7 +187,7 @@ void dst_release_immediate(struct dst_entry *dst)
>  		int newrefcnt;
>  
>  		newrefcnt = atomic_dec_return(&dst->__refcnt);
> -		if (unlikely(newrefcnt < 0))
> +		if (WARN_ONCE(newrefcnt < 0, "dst_release_immediate underflow"))
>  			net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
>  					     __func__, dst, newrefcnt);
>  		if (!newrefcnt)
> -- 
> 2.21.0
> 

-- 
Kees Cook