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:
 <SYBPR01MB7881D2110FD5E269FF682327AFC5A@SYBPR01MB7881.ausprd01.prod.outlook.com>
Date: Wed, 5 Nov 2025 04:28:27 +0000
From: 222 Summ <moonafterrain@...look.com>
To: Takashi Iwai <tiwai@...e.de>
CC: Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
	"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>, Yuhao Jiang
	<danisjiang@...il.com>
Subject: Re: [PATCH v2] ALSA: wavefront: use scnprintf for longname
 construction

Hi Takashi,

Thank you for your detailed feedback on the v2 patch. I've prepared a v3
patch series that incorporates your suggestions.

Based on your comments, I've made the following changes:

1. Split the patch into two parts:
   - Patch 1/2: Adds `scnprintf_append()` to `lib/vsprintf.c`
   - Patch 2/2: Converts `wavefront.c` to use it
2. Fixed the return value you pointed out
3. Used strnlen() instead of strlen() for safety

I plan to submit this as a two-patch series. However, before I send it, I'd like to confirm a few things:

1. Is this approach (adding the helper to lib/vsprintf.c) what you had in
   mind? Or would you prefer a different location?
2. Would you recommend sending both patches together, or waiting until
patch 1/2 is reviewed and accepted before submitting patch 2/2?

The implementation of scnprintf_append() is shown below:
+int scnprintf_append(char *buf, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	size_t len;
+
+	len = strnlen(buf, size);
+	if (len >= size)
+		return len;
+	va_start(args, fmt);
+	len += vscnprintf(buf + len, size - len, fmt, args);
+	va_end(args);
+	return len;
+}

Thanks again for your guidance.

Best regards,
Junrui

________________________________________
From: Takashi Iwai <tiwai@...e.de>
Sent: Tuesday, November 4, 2025 18:01
To: moonafterrain@...look.com <moonafterrain@...look.com>
Cc: Jaroslav Kysela <perex@...ex.cz>; Takashi Iwai <tiwai@...e.com>; linux-sound@...r.kernel.org <linux-sound@...r.kernel.org>; linux-kernel@...r.kernel.org <linux-kernel@...r.kernel.org>; stable@...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