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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <201911131625.8B0F0BAEDE@keescook>
Date:   Wed, 13 Nov 2019 16:31:40 -0800
From:   Kees Cook <keescook@...omium.org>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: print proper warning on dst underflow

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@...c4.com>
> ---
>  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