linux-kernel - Re: [PATCH] s390/sysinfo: Replace sprintf with snprintf for buffer safety

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: <20251002074821.7570A92-hca@linux.ibm.com>
Date: Thu, 2 Oct 2025 09:48:21 +0200
From: Heiko Carstens <hca@...ux.ibm.com>
To: Josephine Pfeiffer <hi@...ie.lol>
Cc: Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle <svens@...ux.ibm.com>, linux-s390@...r.kernel.org,
        linux-kernel@...r.kernel.org, Kees Cook <kees@...nel.org>
Subject: Re: [PATCH] s390/sysinfo: Replace sprintf with snprintf for buffer
 safety

On Wed, Oct 01, 2025 at 07:41:04PM +0200, Josephine Pfeiffer wrote:
> Replace sprintf() with snprintf() when formatting symlink target name
> to prevent potential buffer overflow. The link_to buffer is only 10
> bytes, and using snprintf() ensures proper bounds checking if the
> topology nesting limit value is unexpectedly large.
> 
> Signed-off-by: Josephine Pfeiffer <hi@...ie.lol>
> ---
>  arch/s390/kernel/sysinfo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
> index 1ea84e942bd4..33ca3e47a0e6 100644
> --- a/arch/s390/kernel/sysinfo.c
> +++ b/arch/s390/kernel/sysinfo.c
> @@ -526,7 +526,7 @@ static __init int stsi_init_debugfs(void)
>  	if (IS_ENABLED(CONFIG_SCHED_TOPOLOGY) && cpu_has_topology()) {
>  		char link_to[10];
>  
> -		sprintf(link_to, "15_1_%d", topology_mnest_limit());
> +		snprintf(link_to, sizeof(link_to), "15_1_%d", topology_mnest_limit());

[Adding Kees]

I don't think that patches like this will make the world a better
place. But you could try some macro magic and try to figure out if the
first parameter of sprintf() is an array, and if so change the call from
sprintf() to snprintf() transparently for all users. Some similar magic
that has been added to strscpy() with the optional third parameter.

No idea if that is possible at all, or if that would introduce some
breakage.