* [PATCH] ALSA: asihpi: use scnprintf() for control name formatting [not found] <20260323070310.55878-1-pengpeng@iscas.ac.cn> @ 2026-03-27 23:48 ` Pengpeng Hou 2026-03-28 9:52 ` Takashi Iwai 2026-03-28 10:28 ` [PATCH v3] ALSA: asihpi: detect truncated control names Pengpeng Hou 1 sibling, 1 reply; 4+ messages in thread From: Pengpeng Hou @ 2026-03-27 23:48 UTC (permalink / raw) To: perex; +Cc: tiwai, linux-sound, linux-kernel, pengpeng asihpi_ctl_init() builds mixer control names in the fixed 44-byte hpi_ctl->name buffer with sprintf(). This is not only a defensive cleanup. The current in-tree name tables and format strings can already exceed 44 bytes. For example, "Bitstream 0 Internal 0 Monitor Playback Volume" is 46 characters before the trailing NUL, so the current sprintf() call writes past the end of hpi_ctl->name. Switch the formatting to scnprintf() so the driver truncates control names instead of overflowing the fixed ALSA control name storage. Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn> --- sound/pci/asihpi/asihpi.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c index 3a64d0562803..de2859eb3e9f 100644 --- a/sound/pci/asihpi/asihpi.c +++ b/sound/pci/asihpi/asihpi.c @@ -1384,22 +1384,25 @@ static void asihpi_ctl_init(struct snd_kcontrol_new *snd_control, dir = "Playback "; /* PCM Playback source, or output node */ if (hpi_ctl->src_node_type && hpi_ctl->dst_node_type) - sprintf(hpi_ctl->name, "%s %d %s %d %s%s", - asihpi_src_names[hpi_ctl->src_node_type], - hpi_ctl->src_node_index, - asihpi_dst_names[hpi_ctl->dst_node_type], - hpi_ctl->dst_node_index, - dir, name); + scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name), + "%s %d %s %d %s%s", + asihpi_src_names[hpi_ctl->src_node_type], + hpi_ctl->src_node_index, + asihpi_dst_names[hpi_ctl->dst_node_type], + hpi_ctl->dst_node_index, + dir, name); else if (hpi_ctl->dst_node_type) { - sprintf(hpi_ctl->name, "%s %d %s%s", - asihpi_dst_names[hpi_ctl->dst_node_type], - hpi_ctl->dst_node_index, - dir, name); + scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name), + "%s %d %s%s", + asihpi_dst_names[hpi_ctl->dst_node_type], + hpi_ctl->dst_node_index, + dir, name); } else { - sprintf(hpi_ctl->name, "%s %d %s%s", - asihpi_src_names[hpi_ctl->src_node_type], - hpi_ctl->src_node_index, - dir, name); + scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name), + "%s %d %s%s", + asihpi_src_names[hpi_ctl->src_node_type], + hpi_ctl->src_node_index, + dir, name); } } -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: asihpi: use scnprintf() for control name formatting 2026-03-27 23:48 ` [PATCH] ALSA: asihpi: use scnprintf() for control name formatting Pengpeng Hou @ 2026-03-28 9:52 ` Takashi Iwai 0 siblings, 0 replies; 4+ messages in thread From: Takashi Iwai @ 2026-03-28 9:52 UTC (permalink / raw) To: Pengpeng Hou; +Cc: perex, tiwai, linux-sound, linux-kernel On Sat, 28 Mar 2026 00:48:48 +0100, Pengpeng Hou wrote: > > asihpi_ctl_init() builds mixer control names in the fixed 44-byte > hpi_ctl->name buffer with sprintf(). > > This is not only a defensive cleanup. The current in-tree name tables and > format strings can already exceed 44 bytes. For example, > > "Bitstream 0 Internal 0 Monitor Playback Volume" > > is 46 characters before the trailing NUL, so the current sprintf() call > writes past the end of hpi_ctl->name. > > Switch the formatting to scnprintf() so the driver truncates control > names instead of overflowing the fixed ALSA control name storage. > > Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn> Thanks, now it's clearer. But if the string overflow can happen really, it's rather a bigger problem; then we can't blindly truncate the string as it's used as the control API element key. I guess this didn't happen in reality because the choice of source and destination depends on the firmware and the selection like the above didn't exist so far in the provided firmware data. But, we still should address the problem, of course. Anyways, if the truncation happens, we'd rather need to notify users that it's basically an error. That is, we should use snprintf() in this case and have a return value check. If it's truncated, the driver spews the error, showing the truncated string. Could you revise the patch in that way and resubmit v3? thanks, Takashi > --- > sound/pci/asihpi/asihpi.c | 31 +++++++++++++++++-------------- > 1 file changed, 17 insertions(+), 14 deletions(-) > > diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c > index 3a64d0562803..de2859eb3e9f 100644 > --- a/sound/pci/asihpi/asihpi.c > +++ b/sound/pci/asihpi/asihpi.c > @@ -1384,22 +1384,25 @@ static void asihpi_ctl_init(struct snd_kcontrol_new *snd_control, > dir = "Playback "; /* PCM Playback source, or output node */ > > if (hpi_ctl->src_node_type && hpi_ctl->dst_node_type) > - sprintf(hpi_ctl->name, "%s %d %s %d %s%s", > - asihpi_src_names[hpi_ctl->src_node_type], > - hpi_ctl->src_node_index, > - asihpi_dst_names[hpi_ctl->dst_node_type], > - hpi_ctl->dst_node_index, > - dir, name); > + scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name), > + "%s %d %s %d %s%s", > + asihpi_src_names[hpi_ctl->src_node_type], > + hpi_ctl->src_node_index, > + asihpi_dst_names[hpi_ctl->dst_node_type], > + hpi_ctl->dst_node_index, > + dir, name); > else if (hpi_ctl->dst_node_type) { > - sprintf(hpi_ctl->name, "%s %d %s%s", > - asihpi_dst_names[hpi_ctl->dst_node_type], > - hpi_ctl->dst_node_index, > - dir, name); > + scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name), > + "%s %d %s%s", > + asihpi_dst_names[hpi_ctl->dst_node_type], > + hpi_ctl->dst_node_index, > + dir, name); > } else { > - sprintf(hpi_ctl->name, "%s %d %s%s", > - asihpi_src_names[hpi_ctl->src_node_type], > - hpi_ctl->src_node_index, > - dir, name); > + scnprintf(hpi_ctl->name, sizeof(hpi_ctl->name), > + "%s %d %s%s", > + asihpi_src_names[hpi_ctl->src_node_type], > + hpi_ctl->src_node_index, > + dir, name); > } > } > > -- > 2.50.1 (Apple Git-155) > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3] ALSA: asihpi: detect truncated control names [not found] <20260323070310.55878-1-pengpeng@iscas.ac.cn> 2026-03-27 23:48 ` [PATCH] ALSA: asihpi: use scnprintf() for control name formatting Pengpeng Hou @ 2026-03-28 10:28 ` Pengpeng Hou 2026-03-28 13:14 ` Takashi Iwai 1 sibling, 1 reply; 4+ messages in thread From: Pengpeng Hou @ 2026-03-28 10:28 UTC (permalink / raw) To: perex; +Cc: tiwai, linux-sound, linux-kernel, pengpeng asihpi_ctl_init() builds mixer control names in the fixed 44-byte hpi_ctl->name buffer with sprintf(). This is not only a defensive cleanup. The current in-tree name tables and format strings can already exceed 44 bytes. For example, "Bitstream 0 Internal 0 Monitor Playback Volume" is 46 characters before the trailing NUL, so the current sprintf() call writes past the end of hpi_ctl->name. The generated control name is used as the ALSA control element key, so blindly truncating it is not sufficient. Switch the formatting to snprintf() and emit an error if truncation happens, showing the truncated name while still keeping the write bounded to hpi_ctl->name. Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn> --- v3: - use snprintf() instead of scnprintf() - check the return value and report truncated control names v2: - clarify that the current in-tree strings can already overflow the buffer - use scnprintf() instead of sprintf() - drop the unrelated trailing whitespace change sound/pci/asihpi/asihpi.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c index 3a64d0562803..b7010d83e89c 100644 --- a/sound/pci/asihpi/asihpi.c +++ b/sound/pci/asihpi/asihpi.c @@ -1362,6 +1362,7 @@ static void asihpi_ctl_init(struct snd_kcontrol_new *snd_control, struct hpi_control *hpi_ctl, char *name) { + int len; char *dir; memset(snd_control, 0, sizeof(*snd_control)); snd_control->name = hpi_ctl->name; @@ -1384,23 +1385,30 @@ static void asihpi_ctl_init(struct snd_kcontrol_new *snd_control, dir = "Playback "; /* PCM Playback source, or output node */ if (hpi_ctl->src_node_type && hpi_ctl->dst_node_type) - sprintf(hpi_ctl->name, "%s %d %s %d %s%s", - asihpi_src_names[hpi_ctl->src_node_type], - hpi_ctl->src_node_index, - asihpi_dst_names[hpi_ctl->dst_node_type], - hpi_ctl->dst_node_index, - dir, name); + len = snprintf(hpi_ctl->name, sizeof(hpi_ctl->name), + "%s %d %s %d %s%s", + asihpi_src_names[hpi_ctl->src_node_type], + hpi_ctl->src_node_index, + asihpi_dst_names[hpi_ctl->dst_node_type], + hpi_ctl->dst_node_index, + dir, name); else if (hpi_ctl->dst_node_type) { - sprintf(hpi_ctl->name, "%s %d %s%s", - asihpi_dst_names[hpi_ctl->dst_node_type], - hpi_ctl->dst_node_index, - dir, name); + len = snprintf(hpi_ctl->name, sizeof(hpi_ctl->name), + "%s %d %s%s", + asihpi_dst_names[hpi_ctl->dst_node_type], + hpi_ctl->dst_node_index, + dir, name); } else { - sprintf(hpi_ctl->name, "%s %d %s%s", - asihpi_src_names[hpi_ctl->src_node_type], - hpi_ctl->src_node_index, - dir, name); + len = snprintf(hpi_ctl->name, sizeof(hpi_ctl->name), + "%s %d %s%s", + asihpi_src_names[hpi_ctl->src_node_type], + hpi_ctl->src_node_index, + dir, name); } + + if (len >= sizeof(hpi_ctl->name)) + pr_err("asihpi: truncated control name: %s\n", + hpi_ctl->name); } /*------------------------------------------------------------ -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] ALSA: asihpi: detect truncated control names 2026-03-28 10:28 ` [PATCH v3] ALSA: asihpi: detect truncated control names Pengpeng Hou @ 2026-03-28 13:14 ` Takashi Iwai 0 siblings, 0 replies; 4+ messages in thread From: Takashi Iwai @ 2026-03-28 13:14 UTC (permalink / raw) To: Pengpeng Hou; +Cc: perex, tiwai, linux-sound, linux-kernel On Sat, 28 Mar 2026 11:28:08 +0100, Pengpeng Hou wrote: > > asihpi_ctl_init() builds mixer control names in the fixed 44-byte > hpi_ctl->name buffer with sprintf(). > > This is not only a defensive cleanup. The current in-tree name tables and > format strings can already exceed 44 bytes. For example, > > "Bitstream 0 Internal 0 Monitor Playback Volume" > > is 46 characters before the trailing NUL, so the current sprintf() call > writes past the end of hpi_ctl->name. > > The generated control name is used as the ALSA control element key, so > blindly truncating it is not sufficient. Switch the formatting to > snprintf() and emit an error if truncation happens, showing the > truncated name while still keeping the write bounded to hpi_ctl->name. > > Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn> Applied to for-next branch now. Thanks. Takashi ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-28 13:14 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260323070310.55878-1-pengpeng@iscas.ac.cn>
2026-03-27 23:48 ` [PATCH] ALSA: asihpi: use scnprintf() for control name formatting Pengpeng Hou
2026-03-28 9:52 ` Takashi Iwai
2026-03-28 10:28 ` [PATCH v3] ALSA: asihpi: detect truncated control names Pengpeng Hou
2026-03-28 13:14 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox