[PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[Mahad Ibrahim]

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

[PATCH 1/7] ALSA: ump: replace strlcat() with strscpy()

[Mahad Ibrahim]

When several function blocks map to the same group, their names are
joined with ", " and the separator was appended with strlcat().

Take the current length of group->name with strlen() and write the
separator at that offset with strscpy(), passing the space that is
left in the buffer.

group->name is NUL-terminated within its array, so the offset is
always less than the array size and at least one byte remains for
strscpy() to work with. Both functions stop at the end of the buffer,
so the string that comes out is the same.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 sound/core/ump.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sound/core/ump.c b/sound/core/ump.c
index 632c13baf21e..c6f55d2b8dc4 100644
--- a/sound/core/ump.c
+++ b/sound/core/ump.c
@@ -597,8 +597,11 @@ void snd_ump_update_group_attrs(struct snd_ump_endpoint *ump)
 			}
 			if (!*fb->info.name)
 				continue;
-			if (*group->name)
-				strlcat(group->name, ", ", sizeof(group->name));
+			if (*group->name) {
+				int len = strlen(group->name);
+
+				strscpy(group->name + len, ", ", sizeof(group->name) - len);
+			}
 			safe_append_string(group->name, sizeof(group->name),
 					   fb->info.name, sizeof(fb->info.name));
 		}
-- 
2.54.0

[PATCH 2/7] ALSA: ac97: replace strlcat() with scnprintf()

[Mahad Ibrahim]

At this point name holds the vendor name written by the earlier
strscpy(). A space and then the codec name were appended with two
strlcat() calls. The else branch below already took its own offset
with strlen() before calling snprintf().

Take that offset once, before the branch, and use it for both. The
two appends become a single scnprintf() writing " %s" at the offset,
and the else branch uses the same variable instead of computing its
own.

The bytes written and the point at which the result is truncated are
unchanged.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 sound/pci/ac97/ac97_codec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
index 0bb65be021d9..dfd712ef0c63 100644
--- a/sound/pci/ac97/ac97_codec.c
+++ b/sound/pci/ac97/ac97_codec.c
@@ -1832,6 +1832,7 @@ void snd_ac97_get_name(struct snd_ac97 *ac97, unsigned int id, char *name,
 		       size_t maxlen, int modem)
 {
 	const struct ac97_codec_id *pid;
+	int len;
 
 	sprintf(name, "0x%x %c%c%c", id,
 		printable(id >> 24),
@@ -1849,9 +1850,9 @@ void snd_ac97_get_name(struct snd_ac97 *ac97, unsigned int id, char *name,
 	} 
 
 	pid = look_for_codec_id(snd_ac97_codec_ids, id);
+	len = strlen(name);
 	if (pid) {
-		strlcat(name, " ", maxlen);
-		strlcat(name, pid->name, maxlen);
+		scnprintf(name + len, maxlen - len, " %s", pid->name);
 		if (pid->mask != 0xffffffff)
 			sprintf(name + strlen(name), " rev %u", id & ~pid->mask);
 		if (ac97 && pid->patch) {
@@ -1860,8 +1861,7 @@ void snd_ac97_get_name(struct snd_ac97 *ac97, unsigned int id, char *name,
 				pid->patch(ac97);
 		}
 	} else {
-		int l = strlen(name);
-		snprintf(name + l, maxlen - l, " id %x", id & 0xff);
+		snprintf(name + len, maxlen - len, " id %x", id & 0xff);
 	}
 }
 
-- 
2.54.0

[PATCH 3/7] ALSA: cmipci: replace strlcat() with strscpy()

[Mahad Ibrahim]

The "-SWIEC" suffix on card->driver was appended with strlcat().

Take the length of the existing driver name and write the suffix at
that offset with strscpy(). This is the same thing the can_multi_ch
branch immediately above already does, so both arms of the
conditional now build the name the same way.

strlcat() and strscpy() both truncate at the end of the buffer, and
the suffix is written at the same offset either way.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 sound/pci/cmipci.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
index 9d9f784e3a8c..c8d98d76b689 100644
--- a/sound/pci/cmipci.c
+++ b/sound/pci/cmipci.c
@@ -2983,8 +2983,12 @@ static int snd_cmipci_create(struct snd_card *card, struct pci_dev *pci,
 		int l = strlen(cm->card->driver);
 		scnprintf(cm->card->driver + l, sizeof(cm->card->driver) - l,
 			  "-MC%d", cm->max_channels);
-	} else if (cm->can_ac3_sw)
-		strlcat(cm->card->driver, "-SWIEC", sizeof(cm->card->driver));
+	} else if (cm->can_ac3_sw) {
+		int l = strlen(cm->card->driver);
+
+		strscpy(cm->card->driver + l, "-SWIEC",
+			sizeof(cm->card->driver) - l);
+	}
 
 	cm->dig_status = SNDRV_PCM_DEFAULT_CON_SPDIF;
 	cm->dig_pcm_status = SNDRV_PCM_DEFAULT_CON_SPDIF;
-- 
2.54.0

[PATCH 4/7] ALSA: caiaq: replace strlcat() with strscpy()

[Mahad Ibrahim]

usb_make_path() fills cdev->phys with the device path, and "/input0"
was then appended with strlcat().

Take the length of the path with strlen() and write the suffix at
that offset with strscpy(), passing the remaining space in the
buffer.

cdev->phys is NUL-terminated by usb_make_path(), so the offset is
within the array and the size passed to strscpy() is at least one.
The suffix lands in the same place and truncates at the same point.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 sound/usb/caiaq/input.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/usb/caiaq/input.c b/sound/usb/caiaq/input.c
index 8d924330c54c..fd4c80c7530c 100644
--- a/sound/usb/caiaq/input.c
+++ b/sound/usb/caiaq/input.c
@@ -604,14 +604,15 @@ int snd_usb_caiaq_input_init(struct snd_usb_caiaqdev *cdev)
 {
 	struct usb_device *usb_dev = cdev->chip.dev;
 	struct input_dev *input;
-	int i, ret = 0;
+	int i, len, ret = 0;
 
 	input = input_allocate_device();
 	if (!input)
 		return -ENOMEM;
 
 	usb_make_path(usb_dev, cdev->phys, sizeof(cdev->phys));
-	strlcat(cdev->phys, "/input0", sizeof(cdev->phys));
+	len = strlen(cdev->phys);
+	strscpy(cdev->phys + len, "/input0", sizeof(cdev->phys) - len);
 
 	input->name = cdev->product_name;
 	input->phys = cdev->phys;
-- 
2.54.0

[PATCH 5/7] ALSA: usb-audio: replace strlcat() with append_ctl_name()

[Mahad Ibrahim]

append_ctl_name() appended to kctl->id.name with strlcat() and
returned its result. build_connector_control() open-coded the same
append for the " Jack" suffix rather than calling the helper.

Take the length of the existing name and write the suffix at that
offset with strscpy(). The return value is rebuilt as the offset plus
the length of the appended string, which is what strlcat() returns:
the length the caller asked for, whether or not it fit. No caller
currently uses it. While here, call append_ctl_name() for the " Jack"
suffix instead of repeating the append inline.

The name that ends up in kctl->id.name is unchanged, and so is the
value returned to callers.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 sound/usb/mixer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index 703c118f9d4e..77335bc89aa0 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1748,7 +1748,11 @@ const struct snd_kcontrol_new *snd_usb_feature_unit_ctl = &usb_feature_unit_ctl;
  */
 static size_t append_ctl_name(struct snd_kcontrol *kctl, const char *str)
 {
-	return strlcat(kctl->id.name, str, sizeof(kctl->id.name));
+	size_t len = strlen(kctl->id.name);
+
+	strscpy(kctl->id.name + len, str, sizeof(kctl->id.name) - len);
+
+	return len + strlen(str);
 }
 
 /*
@@ -2116,7 +2120,7 @@ static void build_connector_control(struct usb_mixer_interface *mixer,
 	}
 
 	if (check_mapped_name(map, kctl->id.name, sizeof(kctl->id.name)))
-		strlcat(kctl->id.name, " Jack", sizeof(kctl->id.name));
+		append_ctl_name(kctl, " Jack");
 	else
 		get_connector_control_name(mixer, term, is_input, kctl->id.name,
 					   sizeof(kctl->id.name));
-- 
2.54.0

[PATCH 6/7] ALSA: hiface: replace strlcat() with scnprintf()

[Mahad Ibrahim]

card->longname was built with two strlcat() calls, one copying
card->shortname and one appending " at ". The return value of the
second gave the offset that usb_make_path() writes at.

card->longname is empty here. snd_card_new() allocates struct
snd_card with kzalloc() and nothing writes longname before this
point, so the first strlcat() is really a copy and the two calls
collapse into one scnprintf().

len now counts the characters actually written rather than the
characters requested, so the bounds check below it is always true and
usb_make_path() is reached even when the name was truncated. In that
case it is given a size of one and writes only the NUL terminator
that scnprintf() already placed there, so longname does not change.
Truncation cannot happen in practice anyway: shortname is 32 bytes
and longname is 80.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 sound/usb/hiface/chip.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/hiface/chip.c b/sound/usb/hiface/chip.c
index bce28f683666..d217fe64eabd 100644
--- a/sound/usb/hiface/chip.c
+++ b/sound/usb/hiface/chip.c
@@ -70,8 +70,8 @@ static int hiface_chip_create(struct usb_interface *intf,
 	else
 		strscpy(card->shortname, "M2Tech generic audio", sizeof(card->shortname));
 
-	strlcat(card->longname, card->shortname, sizeof(card->longname));
-	len = strlcat(card->longname, " at ", sizeof(card->longname));
+	len = scnprintf(card->longname, sizeof(card->longname), "%s at ",
+			card->shortname);
 	if (len < sizeof(card->longname))
 		usb_make_path(device, card->longname + len,
 			      sizeof(card->longname) - len);
-- 
2.54.0

[PATCH 7/7] ALSA: usb-audio: replace strlcat() in longname construction

[Mahad Ibrahim]

card->longname was assembled from a chain of strlcat() calls covering
the vendor or manufacturer name, the short name, the USB path and the
speed suffix. The return value of one of them was reused as the
offset for usb_make_path().

Keep the current length in len and write each piece at that offset,
with scnprintf() where a format is involved and strscpy() for the
plain speed suffixes. len is taken again after strim(), which
shortens the string in place, and again after usb_make_path(), which
writes into the buffer directly. Both would otherwise leave the
offset pointing at the wrong byte.

As in the hiface conversion, len now counts characters written rather
than requested, so the bounds check before usb_make_path() is always
true; in the truncated case it writes only the NUL terminator that is
already there. The strings produced for every combination of vendor,
manufacturer, short name and link speed are byte for byte the same as
before.

Signed-off-by: Mahad Ibrahim <mahad.ibrahim.dev@gmail.com>
---
 sound/usb/card.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 24112e491779..3c15a6862046 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -651,7 +651,7 @@ static void usb_audio_make_longname(struct usb_device *dev,
 	struct snd_card *card = chip->card;
 	const struct usb_audio_device_name *preset;
 	const char *s = NULL;
-	int len;
+	int len = 0;
 
 	preset = lookup_device_name(chip->usb_id);
 
@@ -675,32 +675,39 @@ static void usb_audio_make_longname(struct usb_device *dev,
 
 	if (*card->longname) {
 		strim(card->longname);
+		len = strlen(card->longname);
 		if (*card->longname)
-			strlcat(card->longname, " ", sizeof(card->longname));
+			len += scnprintf(card->longname + len,
+					 sizeof(card->longname) - len, " ");
 	}
 
-	strlcat(card->longname, card->shortname, sizeof(card->longname));
-
-	len = strlcat(card->longname, " at ", sizeof(card->longname));
+	len += scnprintf(card->longname + len, sizeof(card->longname) - len,
+			 "%s at ", card->shortname);
 
 	if (len < sizeof(card->longname))
 		usb_make_path(dev, card->longname + len, sizeof(card->longname) - len);
 
+	len = strlen(card->longname);
 	switch (snd_usb_get_speed(dev)) {
 	case USB_SPEED_LOW:
-		strlcat(card->longname, ", low speed", sizeof(card->longname));
+		strscpy(card->longname + len, ", low speed",
+			sizeof(card->longname) - len);
 		break;
 	case USB_SPEED_FULL:
-		strlcat(card->longname, ", full speed", sizeof(card->longname));
+		strscpy(card->longname + len, ", full speed",
+			sizeof(card->longname) - len);
 		break;
 	case USB_SPEED_HIGH:
-		strlcat(card->longname, ", high speed", sizeof(card->longname));
+		strscpy(card->longname + len, ", high speed",
+			sizeof(card->longname) - len);
 		break;
 	case USB_SPEED_SUPER:
-		strlcat(card->longname, ", super speed", sizeof(card->longname));
+		strscpy(card->longname + len, ", super speed",
+			sizeof(card->longname) - len);
 		break;
 	case USB_SPEED_SUPER_PLUS:
-		strlcat(card->longname, ", super speed plus", sizeof(card->longname));
+		strscpy(card->longname + len, ", super speed plus",
+			sizeof(card->longname) - len);
 		break;
 	default:
 		break;
-- 
2.54.0

Re: [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[Takashi Iwai]

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

Re: [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[David Laight]

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
> 

Re: [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[Mahad Ibrahim]

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

Re: [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[Takashi Iwai]

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

Re: [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[Kees Cook]

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

Re: [PATCH 0/7] ALSA: remove remaining strlcat() users under sound/

[David Laight]

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
>