sound/core/ump.c | 7 +++++-- sound/pci/ac97/ac97_codec.c | 8 ++++---- sound/pci/cmipci.c | 8 ++++++-- sound/usb/caiaq/input.c | 5 +++-- sound/usb/card.c | 27 +++++++++++++++++---------- sound/usb/hiface/chip.c | 4 ++-- sound/usb/mixer.c | 8 ++++++-- 7 files changed, 43 insertions(+), 24 deletions(-)
strlcat() is deprecated and slated for removal once its remaining users are converted. This series converts the sound/ users outside of ASoC. Each site is converted to an explicit offset plus a bounded copy: strscpy() where the appended text has no format specifiers, and scnprintf() where it does or where the resulting length is needed. After this series the only remaining strlcat() user under sound/ is sound/soc/codecs/wm_adsp_fw_find_test.c, which goes via ASoC. Build-tested with allmodconfig and boot-tested on x86_64. The generated strings were checked against the pre-patch code in a userspace harness for empty, whitespace-padded, exact-fit and oversized inputs, and came out identical. No audio hardware was available here, so the drivers themselves have not been exercised at runtime. Mahad Ibrahim (7): ALSA: ump: replace strlcat() with strscpy() ALSA: ac97: replace strlcat() with scnprintf() ALSA: cmipci: replace strlcat() with strscpy() ALSA: caiaq: replace strlcat() with strscpy() ALSA: usb-audio: replace strlcat() with append_ctl_name() ALSA: hiface: replace strlcat() with scnprintf() ALSA: usb-audio: replace strlcat() in longname construction sound/core/ump.c | 7 +++++-- sound/pci/ac97/ac97_codec.c | 8 ++++---- sound/pci/cmipci.c | 8 ++++++-- sound/usb/caiaq/input.c | 5 +++-- sound/usb/card.c | 27 +++++++++++++++++---------- sound/usb/hiface/chip.c | 4 ++-- sound/usb/mixer.c | 8 ++++++-- 7 files changed, 43 insertions(+), 24 deletions(-) -- 2.54.0
On Fri, 07 Aug 2026 13:41:32 +0200, Mahad Ibrahim wrote: > > strlcat() is deprecated and slated for removal once its remaining > users are converted. This series converts the sound/ users outside > of ASoC. > > Each site is converted to an explicit offset plus a bounded copy: > strscpy() where the appended text has no format specifiers, and > scnprintf() where it does or where the resulting length is needed. > > After this series the only remaining strlcat() user under sound/ is > sound/soc/codecs/wm_adsp_fw_find_test.c, which goes via ASoC. > > Build-tested with allmodconfig and boot-tested on x86_64. > > The generated strings were checked against the pre-patch code in a > userspace harness for empty, whitespace-padded, exact-fit and > oversized inputs, and came out identical. No audio hardware was > available here, so the drivers themselves have not been exercised at > runtime. > > Mahad Ibrahim (7): > ALSA: ump: replace strlcat() with strscpy() > ALSA: ac97: replace strlcat() with scnprintf() > ALSA: cmipci: replace strlcat() with strscpy() > ALSA: caiaq: replace strlcat() with strscpy() > ALSA: usb-audio: replace strlcat() with append_ctl_name() > ALSA: hiface: replace strlcat() with scnprintf() > ALSA: usb-audio: replace strlcat() in longname construction Honestly speaking, I'm against those conversions. Why do we have to open-code at each place with strlen()+strscpy()? It's just harder to read than strlcat(), even more error-prone. If an alternative is something like this, we really should reconsider. thanks, Takashi
On Fri Aug 7, 2026 at 5:15 PM PKT, Takashi Iwai wrote: > Honestly speaking, I'm against those conversions. > Why do we have to open-code at each place with strlen()+strscpy()? > It's just harder to read than strlcat(), even more error-prone. > > If an alternative is something like this, we really should reconsider. Thank you for the quick response and feedback. You are right that strlen() + strscpy() is tedious and annoying to read. sound/ already has helpers that do this, but each is local to one file with its own signature: safe_append_string() sound/core/ump.c append_ctl_name() sound/usb/mixer.c hda_append_suffix() sound/hda/common/hda_local.h Each of these functions practice the same string append technique however to slightly different effect. safe_append_string uses safe_copy_string() whose function body is above it in the same file. safe_copy_string() performs analogous to strscpy() however adds a filter which drops non-printable ASCII characters during the copy phase. append_ctl_name() is simply an strlcat wrapper which when transitioned would result in the same strlen() + strscpy(). Only separating feature is that it returns the length of characters that would have been written (not necessarily the actual amount). However none of the callers use its return functionality. hda_append_suffix() is a verbatim copy of strlen() + strscpy(). A solution I would propose is that all these functions which do the same thing, aside from safe_append_string, could be moved where they are accessible globally across sound/. This would remove the redundant need to use strlen() + strscpy() in replacement for strlcat() and would unify the sub-system under a single string append API. safe_append_string is only called once in the entire sub-system, and could be replaced either within the function with the unified string append function, and a separate filterer replacing the safe_copy_string function or removed all together and managed inline within the single caller. However this function would require a more involved removal as the internal safe_copy_string is called twice; it is called once in safe_append_string(), and in sound/core/ump.c for a wrapper function ump_set_rawmidi_name(). An argument against this suggestion is that it would confine a string append helper to a single sub-system, while the rest of the kernel uses something else. Additionally patch 1/7 shouldn't have open-coded anything at all. safe_append_string() was already a few hundred lines above the site I touched, and I should have used it. This was based on https://github.com/KSPP/linux/issues/370, which I should have linked in the cover letter. Thank you for your time. Best regards, Mahad Ibrahim
On Fri, 07 Aug 2026 17:41:36 +0200, Mahad Ibrahim wrote: > > On Fri Aug 7, 2026 at 5:15 PM PKT, Takashi Iwai wrote: > > Honestly speaking, I'm against those conversions. > > Why do we have to open-code at each place with strlen()+strscpy()? > > It's just harder to read than strlcat(), even more error-prone. > > > > If an alternative is something like this, we really should reconsider. > > Thank you for the quick response and feedback. > > You are right that strlen() + strscpy() is tedious and annoying to read. > > sound/ already has helpers that do this, but each is local to one file > with its own signature: > > safe_append_string() sound/core/ump.c > append_ctl_name() sound/usb/mixer.c > hda_append_suffix() sound/hda/common/hda_local.h > > Each of these functions practice the same string append technique > however to slightly different effect. safe_append_string uses > safe_copy_string() whose function body is above it in the same file. > safe_copy_string() performs analogous to strscpy() however adds a > filter which drops non-printable ASCII characters during the copy phase. > > append_ctl_name() is simply an strlcat wrapper which when transitioned > would result in the same strlen() + strscpy(). Only separating feature > is that it returns the length of characters that would have been > written (not necessarily the actual amount). However none of the callers > use its return functionality. > > hda_append_suffix() is a verbatim copy of strlen() + strscpy(). > > A solution I would propose is that all these functions which do the same > thing, aside from safe_append_string, could be moved where they are > accessible globally across sound/. This would remove the redundant need > to use strlen() + strscpy() in replacement for strlcat() and would unify > the sub-system under a single string append API. > > safe_append_string is only called once in the entire sub-system, and could > be replaced either within the function with the unified string > append function, and a separate filterer replacing the safe_copy_string > function or removed all together and managed inline within the single > caller. However this function would require a more involved removal as the > internal safe_copy_string is called twice; it is called once in > safe_append_string(), and in sound/core/ump.c for a wrapper function > ump_set_rawmidi_name(). > > An argument against this suggestion is that it would confine a string > append helper to a single sub-system, while the rest of the kernel uses > something else. > > Additionally patch 1/7 shouldn't have open-coded anything at all. > safe_append_string() was already a few hundred lines above the site I > touched, and I should have used it. > > This was based on https://github.com/KSPP/linux/issues/370, which I > should have linked in the cover letter. > > Thank you for your time. > > Best regards, > Mahad Ibrahim Well, that leads to a basic question: why do we have to drop strlcat() if most of callers would just need the equivalent function. If strlcat() were super-dangerous, it's understandable to drop. But, it's not, and issues discussed in the github are minor and something that can be addressed in strlcat() implementation; that is, can't we rather re-implement strlcat() in a safer way, instead of killing it? Sure, there are code calling strlcat() that could be optimized better. They can be cleaned up. But it alone can't be a reason that strlcat() must die without mercy. thanks, Takashi
On Fri, Aug 07, 2026 at 06:03:58PM +0200, Takashi Iwai wrote: > If strlcat() were super-dangerous, it's understandable to drop. But, > it's not, and issues discussed in the github are minor and something > that can be addressed in strlcat() implementation; that is, can't we > rather re-implement strlcat() in a safer way, instead of killing it? > > Sure, there are code calling strlcat() that could be optimized better. > They can be cleaned up. But it alone can't be a reason that strlcat() > must die without mercy. The risk comes from the compiler having no way to know what the size of the destination buffer is, as the "char *" argument has no length associated with it. One thing we can do is change the argument requirements for strlcat (like we did when designing memtostr, etc), that requires that the argument explicitly be an array (not a string pointer), at which point bounds checking can be done. Usually this requires changing the plumbing of arguments, as a lot of C code is used to just passing around a bare "char *", etc. And if that re-plumbing is going to happen, it might as well be seq_buf. But yes, just replacing it with strlen/strscpy isn't very ergonomic. Adding the length explicitly with strscpy certainly gets us the bounds again, but it's _separate_ from the string still, and that will lead to mistakes too. Better to have it be part of the type (i.e. either an array or seq_buf). -Kees -- Kees Cook
On Fri, 7 Aug 2026 14:46:44 -0700 Kees Cook <kees@kernel.org> wrote: > On Fri, Aug 07, 2026 at 06:03:58PM +0200, Takashi Iwai wrote: > > If strlcat() were super-dangerous, it's understandable to drop. But, > > it's not, and issues discussed in the github are minor and something > > that can be addressed in strlcat() implementation; that is, can't we > > rather re-implement strlcat() in a safer way, instead of killing it? > > > > Sure, there are code calling strlcat() that could be optimized better. > > They can be cleaned up. But it alone can't be a reason that strlcat() > > must die without mercy. > > The risk comes from the compiler having no way to know what the size of > the destination buffer is, as the "char *" argument has no length > associated with it. One thing we can do is change the argument > requirements for strlcat (like we did when designing memtostr, etc), > that requires that the argument explicitly be an array (not a string > pointer), at which point bounds checking can be done. > > Usually this requires changing the plumbing of arguments, as a lot of C > code is used to just passing around a bare "char *", etc. And if that > re-plumbing is going to happen, it might as well be seq_buf. > > But yes, just replacing it with strlen/strscpy isn't very ergonomic. > Adding the length explicitly with strscpy certainly gets us the bounds > again, but it's _separate_ from the string still, and that will lead to > mistakes too. Better to have it be part of the type (i.e. either an > array or seq_buf). And, if the destination is an array (where the compiler knows the size) there is nothing wrong with a 2 argument function. Like strscpy() you want any result to be the new length of the destination string. Embedding a fixed length char[] in a struct can be a simple better option and lets the compiler do a lot of the checks for you. David > > -Kees >
On Fri, 07 Aug 2026 14:15:27 +0200 Takashi Iwai <tiwai@suse.de> wrote: > On Fri, 07 Aug 2026 13:41:32 +0200, > Mahad Ibrahim wrote: > > > > strlcat() is deprecated and slated for removal once its remaining > > users are converted. This series converts the sound/ users outside > > of ASoC. > > > > Each site is converted to an explicit offset plus a bounded copy: > > strscpy() where the appended text has no format specifiers, and > > scnprintf() where it does or where the resulting length is needed. > > > > After this series the only remaining strlcat() user under sound/ is > > sound/soc/codecs/wm_adsp_fw_find_test.c, which goes via ASoC. > > > > Build-tested with allmodconfig and boot-tested on x86_64. > > > > The generated strings were checked against the pre-patch code in a > > userspace harness for empty, whitespace-padded, exact-fit and > > oversized inputs, and came out identical. No audio hardware was > > available here, so the drivers themselves have not been exercised at > > runtime. > > > > Mahad Ibrahim (7): > > ALSA: ump: replace strlcat() with strscpy() > > ALSA: ac97: replace strlcat() with scnprintf() > > ALSA: cmipci: replace strlcat() with strscpy() > > ALSA: caiaq: replace strlcat() with strscpy() > > ALSA: usb-audio: replace strlcat() with append_ctl_name() > > ALSA: hiface: replace strlcat() with scnprintf() > > ALSA: usb-audio: replace strlcat() in longname construction > > Honestly speaking, I'm against those conversions. > Why do we have to open-code at each place with strlen()+strscpy()? > It's just harder to read than strlcat(), even more error-prone. > > If an alternative is something like this, we really should reconsider. Agreed. Changing the code to not need strcat() is one thing, repeatedly implementing a different version is silly. Even using seq_buf isn't always ideal. David > > > thanks, > > Takashi >
© 2016 - 2026 Red Hat, Inc.