linux-kernel - Re: [PATCH v2] ALSA: wavefront: use scnprintf for longname construction

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: <874irai0ag.wl-tiwai@suse.de>
Date: Tue, 04 Nov 2025 11:01:43 +0100
From: Takashi Iwai <tiwai@...e.de>
To: moonafterrain@...look.com
Cc: Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>,
	linux-sound@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	stable@...r.kernel.org,
	Yuhao Jiang <danisjiang@...il.com>
Subject: Re: [PATCH v2] ALSA: wavefront: use scnprintf for longname construction

On Sun, 02 Nov 2025 16:32:39 +0100,
moonafterrain@...look.com wrote:
> 
> From: Junrui Luo <moonafterrain@...look.com>
> 
> Replace sprintf() calls with scnprintf() and a new scnprintf_append()
> helper function when constructing card->longname. This improves code
> readability and provides bounds checking for the 80-byte buffer.
> 
> While the current parameter ranges don't cause overflow in practice,
> using safer string functions follows kernel best practices and makes
> the code more maintainable.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@...r.kernel.org
> Signed-off-by: Junrui Luo <moonafterrain@...look.com>
> ---
> Changes in v2:
> - Replace sprintf() calls with scnprintf() and a new scnprintf_append()
> - Link to v1: https://lore.kernel.org/all/ME2PR01MB3156CEC4F31F253C9B540FB7AFFDA@ME2PR01MB3156.ausprd01.prod.outlook.com/

Well, my suggestion was that we can apply such conversions once if a
*generic* helper becomes available; that is, propose
scnprintf_append() to be put in include/linux/string.h or whatever (I
guess better in *.c instead of inline), and once if it's accepted, we
can convert the relevant places (there are many, not only
wavefront.c).

BTW:

> +__printf(3, 4) static int scnprintf_append(char *buf, size_t size, const char *fmt, ...)
> +{
> +	va_list args;
> +	size_t len = strlen(buf);
> +
> +	if (len >= size)
> +		return len;
> +	va_start(args, fmt);
> +	len = vscnprintf(buf + len, size - len, fmt, args);
> +	va_end(args);
> +	return len;

The above should be
	len += vscnprintf(buf + len, size - len, fmt, args);
so that it returns the full size of the string.
If it were in user-space, I'd check a negative error code, but the
Linux kernel implementation doesn't return a negative error code, so
far.
I see it's a copy from a code snipped I suggested which already
contained the error :)

Also, it might be safer to use strnlen() instead of strlen() for
avoiding a potential out-of-bound access.


thanks,

Takashi