netdev - Re: [PATCH net-next] net/rds: Replace deprecated strncpy() with strscpy

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <202502191855.C9B9A7AA@keescook>
Date: Wed, 19 Feb 2025 18:57:01 -0800
From: Kees Cook <kees@...nel.org>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: Allison Henderson <allison.henderson@...cle.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, linux-hardening@...r.kernel.org,
	netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
	rds-devel@....oracle.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next] net/rds: Replace deprecated strncpy() with
 strscpy_pad()

On Wed, Feb 19, 2025 at 11:47:31PM +0100, Thorsten Blum wrote:
> strncpy() is deprecated for NUL-terminated destination buffers. Use
> strscpy_pad() instead and remove the manual NUL-termination.

When doing these conversions, please describe two aspects of
conversions:

- Why is it safe to be NUL terminated
- Why is it safe to be/not-be NUL-padded

In this case, the latter needs examination. Looking at how ctr is used,
it is memcpy()ed later, which means this string MUST be NUL padded or it
will leak stack memory contents.

So, please use strscpy_pad() here. :)

-Kees

> 
> Compile-tested only.
> 
> Link: https://github.com/KSPP/linux/issues/90
> Cc: linux-hardening@...r.kernel.org
> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.dev>
> ---
>  net/rds/stats.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/rds/stats.c b/net/rds/stats.c
> index 9e87da43c004..cb2e3d2cdf73 100644
> --- a/net/rds/stats.c
> +++ b/net/rds/stats.c
> @@ -89,8 +89,7 @@ void rds_stats_info_copy(struct rds_info_iterator *iter,
>  
>  	for (i = 0; i < nr; i++) {
>  		BUG_ON(strlen(names[i]) >= sizeof(ctr.name));
> -		strncpy(ctr.name, names[i], sizeof(ctr.name) - 1);
> -		ctr.name[sizeof(ctr.name) - 1] = '\0';
> +		strscpy_pad(ctr.name, names[i]);
>  		ctr.value = values[i];
>  
>  		rds_info_copy(iter, &ctr, sizeof(ctr));
> -- 
> 2.48.1
> 
> 

-- 
Kees Cook