[RFC PATCH 0/8] ALSA: untruncated card names

[RFC PATCH 0/8] ALSA: untruncated card names

[Luca Rodenhäuser]

This is the series behind the design question from last week, shaped by
what both of you said there.

Link: https://lore.kernel.org/linux-sound/AMBP191MB2886BF4BED02C050FB4D6B76CEBB2@AMBP191MB2886.EURP191.PROD.OUTLOOK.COM/

Takashi:

  "I personally find it OK to have UTF-8 strings in short / long / mixer
   names.  That's no new problem, and it's been over decades.  Of course,
   a broken letter at the end could be a problem and we can fix it in
   either kernel or alsa-lib, too."

Jaroslav:

  "The fields are printed in procfs, so the kernel code should do this."

  "Accepting UTF-8 in fields and provide short-long/long-long name through
   SNDRV_CTL_IOCTL_CARD_BYTES when it does not fit to the standard field
   length is also acceptable (as for the components string)."

So the series does not introduce a second, differently encoded set of name
fields, which is what my first sketch proposed.  It treats the existing
fields as what they are - UTF-8, fixed size - and fixes the two things that
follow from the fixed size.

The problem
-----------

usb_string() converts the UTF-16LE string descriptors of a device into
UTF-8, so dev->product reaches the card name fields as UTF-8 already.
usb_audio_make_shortname() copies it into the 32-byte card->shortname with
strscpy(), which cuts at a byte boundary, so the last sequence can end up
truncated.  What user space receives is then not valid UTF-8 and cannot be
repaired, because the missing bytes never left the kernel.

And cutting cleanly is still cutting: 32 bytes is eight CJK characters.

The series
----------

Two pre-existing fixes first, both in code this series then builds on:

1/8 stops a non-ASCII byte from reaching card->id.  copy_valid_id_string()
    has a path that copies the source byte guarded only by isalpha(), and
    the ctype table classifies the Latin-1 high bytes as letters, so a USB
    product name like "\xc3\x84" "3000" yields an id starting with 0xc3.
    That id becomes a /proc/asound directory name and the hw:CARD= handle,
    and sysfs id_store() rejects exactly what the kernel produces here.

2/8 moves the copy_to_user() in snd_ctl_card_bytes() out of
    snd_ioctl_rwsem.  A user buffer that faults can be made to take
    arbitrarily long, and that lock is global and taken for writing from
    the card probe path.

Then the truncation, which is Takashi's point and, per Jaroslav, belongs in
the kernel because the kernel is what prints these names into procfs:

3/8 adds snd_utf8_strscpy(), which is strscpy() except that it ends the
    copy after the last complete sequence that fits, and uses it in
    usb-audio.  For pure ASCII the output is byte for byte what strscpy()
    produced.  If the string turns out not to be UTF-8 after all, the plain
    cut stands.

    This one stands alone: it is worth applying whether or not the rest of
    the series goes anywhere, and nothing later depends on it.

Then the length, in the shape Jaroslav described:

4/8 adds card->full_shortname and friends, holding the whole name,
    allocated on demand, NULL when the name fit - exactly as
    card->components holds the whole components string of which
    snd_ctl_card_info::components is a cut copy.  "Set" therefore means
    "the fixed field is not the whole story": one name per card, in two
    lengths, never a pair that could disagree.

5/8 returns them as three more SNDRV_CTL_IOCTL_CARD_BYTES types, next to
    the existing _COMPONENTS.  No existing struct or ioctl changes; an old
    kernel answers -EINVAL for an unknown type, which means the same as the
    empty string: the fixed field already holds the whole name.

6/8 adds KUnit coverage.

7/8 makes usb-audio offer them.

8/8 documents it, including what the kernel does not promise.

What stays out of the fixed fields
----------------------------------

Takashi asked what the fixed short/long/mixer names should hold once the
untruncated ones exist - embedded ids like "USB 1234:abcd", or the vendor
and product names when they happen to be ASCII, falling back otherwise.

This series does neither: those fields keep exactly what they hold today,
minus the mid-character cut.  A card that shows up as "Caf\xc3\xa9 Audio"
today is displayed correctly by anything that treats the bytes as UTF-8,
and replacing that with a hex id takes a readable name away.  Falling back
only for non-ASCII names does the same to exactly the devices this is
about, and makes the content depend on a property of the string that user
space cannot predict.  Keeping the fixed fields literally unchanged is also
what makes the untruncated ones safe to add: no existing reader can
regress, because no existing reader sees anything different.

On the id, Jaroslav noted that this is really the only field where
normalisation is needed, since usb-audio hardcodes driver and mixername:

  driver_name: USB-Audio
  mixer_name: USB Mixer

1/8 keeps the id valid, but the result for a non-ASCII name is a fairly
meaningless "D3000".  Deriving it from the USB id instead, as suggested,
would read better, but it changes existing card ids, which udev rules and
hw:CARD= entries point at.  That deserves its own patch and its own
discussion, so it is not in this series; I will send it separately unless
you would rather have it here.

What the kernel promises
------------------------

Two things: a name is well-formed UTF-8 and is never cut inside a sequence,
and it carries no code point that would break a line (C0 and C1 controls,
U+2028, U+2029).  The second one is not cosmetic, since /proc/asound/cards
is line based.

Nothing else.  Invisible and bidirectional format characters, the tag
block, variation selectors and non-characters all pass through, and the
header says so and tells the reader to isolate names for display and to
filter Default_Ignorable_Code_Point itself.

An earlier draft did try to filter those, with a list of ranges.  An
outside review took it apart: U+061C, a real Bidi_Control, fell through, as
did U+2065 and U+206A-206F in the middle of a range the code claimed to
cover, the whole tag block, the variation selectors and U+00AD.  Completing
the list means carrying Unicode character properties in the sound core and
revisiting them every Unicode release, which is the same dependency avoided
by leaving NFC to drivers.  A partial list is worse than none, because the
promise in the header invites user space to skip its own filtering.

The same review found the more basic problem: rejecting a name achieved
nothing, because the fixed field still holds it and readers are told to use
that.  So names are repaired instead - a malformed sequence becomes U+FFFD,
one per bad byte as recommended for a maximal subpart in Unicode TR#36, and
a line breaking code point becomes a space.  The setters cannot fail except
with -ENOMEM.

Still open
----------

1. Limits.  Each untruncated field has a byte cap of four times the
   matching fixed field (128/320/320), so the same number of characters
   always fits.  For card->components the request was to drop the
   artificial cap and let the ioctl report the length.  The same argument
   applies here; say the word and the caps go.

2. full_mixername has no in-tree user, since usb-audio hardcodes
   "USB Mixer".  It is there because the three name fields are one set, but
   if "no user, no API" applies it can be dropped and added with its first
   caller.

3. Should the fixed fields carry the '>' truncation marker that components
   uses, to tell a reader that more is available?  It would be consistent,
   but it changes what every existing reader sees, so I have not done it.

4. Consumers.  alsa-lib and PipeWire have not been asked yet.  If you would
   rather see their interest first, I will do that before a v2.

Testing
-------

18/18 cases of the sound-core-test KUnit suite pass under ARCH=um, also
with CONFIG_KASAN=y.  Two of the new cases are regression tests for the
review findings: one pins that card->id stays ASCII (it fails without 1/8,
with card->id[0] = 0xc3), one pins that the characters the kernel does not
filter really do pass through unchanged, so that nobody later mistakes that
for an oversight.

The sanitiser was fuzzed for 400 000 iterations under ASan and UBSan
against an independent UTF-8 decoder, with the real limits and with limits
small enough that a replacement character cannot fit: no memory errors, no
malformed output, no output over the limit.  The harness includes the
function verbatim from init.c; it can be posted if useful.

sound/ builds warning-free with W=1 on an x86_64 defconfig plus SND,
SND_USB_AUDIO and SND_CORE_TEST.  Each patch builds on its own.
checkpatch --strict is clean apart from warnings about deliberately split
string literals in the test data, where a hex escape is followed by a hex
digit.

The usb-audio patches are tested by build and by inspection only; I have no
device with a non-ASCII product string, so that is the piece that would
benefit from a real test.

Tooling
-------

Per Documentation/process/generated-content.rst: the implementation was
written with the help of an LLM coding assistant, from a prompt describing
the problem and the constraint to fit the existing kernel structure.  The
first draft was then reviewed by a second, independent assistant run that
had not written it, which produced the findings described above; the KUnit
and fuzzing work backing them is that review's, and the series was reworked
in response, and reshaped again after your replies.  Every patch carries an
Assisted-by tag.  I have reviewed the result and stand behind it.

Luca Rodenhäuser (8):
  ALSA: core: keep non-ASCII bytes out of the card id
  ALSA: control: copy the card bytes outside snd_ioctl_rwsem
  ALSA: usb-audio: don't cut the card name inside a UTF-8 character
  ALSA: core: carry the untruncated form of the card names
  ALSA: control: return the untruncated card names via CARD_BYTES
  ALSA: core: add KUnit coverage for the card names
  ALSA: usb-audio: offer the untruncated card names
  ALSA: Documentation: describe the card name fields

 Documentation/sound/designs/card-names.rst    | 178 ++++++++++
 Documentation/sound/designs/index.rst         |   1 +
 .../kernel-api/writing-an-alsa-driver.rst     |  12 +
 include/sound/core.h                          |  28 ++
 include/uapi/sound/asound.h                   |  30 +-
 sound/core/control.c                          |  68 +++-
 sound/core/init.c                             | 333 ++++++++++++++++-
 sound/core/sound_kunit.c                      | 334 ++++++++++++++++++
 sound/usb/card.c                              | 106 ++++--
 9 files changed, 1045 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/sound/designs/card-names.rst

-- 
2.43.0

[RFC PATCH 1/8] ALSA: core: keep non-ASCII bytes out of the card id

[Luca Rodenhäuser]

copy_valid_id_string() filters the source string through safe_ascii_char()
everywhere but one place.  When the name it derives the id from starts with
a digit, it prepends a letter taken from the original string, guarded only
by isalpha():

	if (isdigit(*nid))
		*id++ = isalpha(*src) ? *src : 'D';

The kernel ctype table classifies the Latin-1 high bytes as letters, so
isalpha(0xc3) is true, and that byte is copied into card->id unchanged.

A USB device is enough to trigger it: usb_audio_make_shortname() copies
dev->product into card->shortname, so a product name whose first
ASCII-alphanumeric byte is a digit, such as "\xc3\x84" "3000", leaves
card->id starting with 0xc3.

That matters because card->id is an identifier, not a display name.  It
becomes a directory name under /proc/asound, it is handed to user space in
snd_ctl_card_info::id, and it is the handle in alsa-lib's hw:CARD=<id>.
The sysfs id_store() path rejects anything outside
safe_ascii_char() || '_' || '-', so the kernel currently produces ids that
it refuses to accept from user space.

Guard the byte with isascii() as well.  A non-ASCII first byte then falls
back to 'D', which is what the same line already does for anything that is
not a letter.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 sound/core/init.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/core/init.c b/sound/core/init.c
index 9693e646b3..2b33dd7b42 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -707,8 +707,13 @@ static void copy_valid_id_string(struct snd_card *card, const char *src,
 
 	while (*nid && !safe_ascii_char(*nid))
 		nid++;
-	if (isdigit(*nid))
-		*id++ = isalpha(*src) ? *src : 'D';
+	if (isdigit(*nid)) {
+		/* isalpha() alone is not enough: the ctype table classifies the
+		 * Latin-1 high bytes as letters, so a non-ASCII first byte would
+		 * end up in the id, which must stay alphanumeric ASCII.
+		 */
+		*id++ = isascii(*src) && isalpha(*src) ? *src : 'D';
+	}
 	while (*nid && (size_t)(id - card->id) < sizeof(card->id) - 1) {
 		if (safe_ascii_char(*nid))
 			*id++ = *nid;
-- 
2.43.0

[RFC PATCH 2/8] ALSA: control: copy the card bytes outside snd_ioctl_rwsem

[Luca Rodenhäuser]

snd_ctl_card_bytes() holds snd_ioctl_rwsem for reading across its
copy_to_user().  That lock is global to ALSA and is taken for writing from
the card probe path, by snd_component_add().

A user buffer that faults can be made to take arbitrarily long: with
userfaultfd, or with the target buffer mapped from a FUSE file, an
unprivileged process controls when the fault resolves.  While it holds the
reader, a writer from a probing card queues up behind it, and because rwsem
is writer-fair every later reader queues behind the writer.  Card teardown
then waits on the probe path, so an unprivileged reader can stall the
removal of a card it does not own.

Take a copy of the string under the lock, drop the lock, and copy to user
space afterwards.  The extra allocation is a card name sized string on a
path that is not hot.

While at it, report the required length even when the caller's buffer was
too small.  The -ENOMEM return told the caller to try again with a bigger
buffer without telling it how big, and data_len is what the query form of
this ioctl exists for.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 sound/core/control.c | 46 +++++++++++++++++++++++++++-----------------
 1 file changed, 28 insertions(+), 18 deletions(-)

diff --git a/sound/core/control.c b/sound/core/control.c
index 4199342d4f..e404cb55a5 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -14,6 +14,7 @@
 #include <linux/mm.h>
 #include <linux/math64.h>
 #include <linux/sched/signal.h>
+#include <linux/cleanup.h>
 #include <sound/core.h>
 #include <sound/minors.h>
 #include <sound/info.h>
@@ -904,36 +905,45 @@ static int snd_ctl_card_bytes(struct snd_card *card,
 			      struct snd_ctl_card_bytes *info,
 			      unsigned int __user *data_len_out)
 {
+	char *copy __free(kfree) = NULL;
+	bool too_small = false;
 	unsigned int data_len;
 
-	switch (info->type) {
-	case SND_CTL_CARD_BTYPE_COMPONENTS:
-		scoped_guard(rwsem_read, &snd_ioctl_rwsem) {
-			const char *components = card->components;
-
-			if (!components)
-				components = "";
+	if (info->type != SND_CTL_CARD_BTYPE_COMPONENTS)
+		return -EINVAL;
 
-			data_len = strlen(components) + 1;
+	scoped_guard(rwsem_read, &snd_ioctl_rwsem) {
+		const char *components = card->components;
 
-			if (!info->data || info->data_allocated == 0)
-				break;
+		if (!components)
+			components = "";
 
-			if (info->data_allocated < data_len)
-				return -ENOMEM;
+		data_len = strlen(components) + 1;
 
-			if (copy_to_user(u64_to_user_ptr(info->data), components, data_len))
-				return -EFAULT;
+		if (info->data && info->data_allocated != 0) {
+			if (info->data_allocated < data_len) {
+				too_small = true;
+			} else {
+				copy = kmemdup(components, data_len, GFP_KERNEL);
+				if (!copy)
+					return -ENOMEM;
+			}
 		}
-		break;
-	default:
-		return -EINVAL;
 	}
 
+	/* Copy outside the lock.  A user buffer that faults can be made to take
+	 * arbitrarily long (userfaultfd, FUSE), and snd_ioctl_rwsem is global:
+	 * holding it across the copy lets an unprivileged reader block the card
+	 * probe path, and with it a card teardown waiting on that path.
+	 */
+	if (copy && copy_to_user(u64_to_user_ptr(info->data), copy, data_len))
+		return -EFAULT;
+
+	/* report the required size even when the buffer was too small */
 	if (put_user(data_len, data_len_out))
 		return -EFAULT;
 
-	return 0;
+	return too_small ? -ENOMEM : 0;
 }
 
 static int snd_ctl_card_bytes_user(struct snd_card *card,
-- 
2.43.0

[RFC PATCH 3/8] ALSA: usb-audio: don't cut the card name inside a UTF-8 character

[Luca Rodenhäuser]

usb_string() converts the UTF-16LE string descriptors of a device into
UTF-8, so dev->product may contain multi-byte characters.  Copying it into
the 32-byte card->shortname with strscpy() cuts at a byte boundary, so a
name that does not fit can end in a partial sequence.  What user space
receives is then not valid UTF-8, and it cannot repair it: the missing
bytes never left the kernel.

Add snd_utf8_strscpy(), which behaves exactly like strscpy() except that it
ends the copy after the last complete sequence that fits.  A name that is
too long loses up to three more bytes than before, but what remains is a
whole character.  For pure ASCII the result is byte for byte what strscpy()
produced, so nothing changes for the vast majority of devices.

If the string turns out not to be UTF-8 after all - more continuation bytes
in a row than any sequence can have - the plain strscpy() cut stands, so a
device with a broken descriptor is no worse off than today.

This is worth doing on its own, independently of any richer replacement for
the name fields: it makes the existing field correct for every reader that
is already there, without changing what those readers see for any name that
fits.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 include/sound/core.h     |  1 +
 sound/core/init.c        | 52 +++++++++++++++++++++++++++++++++++
 sound/core/sound_kunit.c | 58 ++++++++++++++++++++++++++++++++++++++++
 sound/usb/card.c         |  7 +++--
 4 files changed, 116 insertions(+), 2 deletions(-)

diff --git a/include/sound/core.h b/include/sound/core.h
index 2ca24ac7e3..36bb95044a 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -316,6 +316,7 @@ void snd_card_free(struct snd_card *card);
 void snd_card_free_when_closed(struct snd_card *card);
 int snd_card_free_on_error(struct device *dev, int ret);
 void snd_card_set_id(struct snd_card *card, const char *id);
+size_t snd_utf8_strscpy(char *dst, const char *src, size_t size);
 int snd_card_register(struct snd_card *card);
 int snd_card_info_init(void);
 int snd_card_add_dev_attr(struct snd_card *card,
diff --git a/sound/core/init.c b/sound/core/init.c
index 2b33dd7b42..67b79b1826 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -789,6 +789,58 @@ void snd_card_set_id(struct snd_card *card, const char *nid)
 }
 EXPORT_SYMBOL(snd_card_set_id);
 
+/**
+ *  snd_utf8_strscpy - copy a UTF-8 string, cutting only at a character boundary
+ *  @dst: destination buffer
+ *  @src: source string, expected to be UTF-8
+ *  @size: size of @dst in bytes
+ *
+ *  Like strscpy(), except that a string too long for @dst ends after the last
+ *  complete UTF-8 sequence that fits, so @dst never ends in the middle of a
+ *  character.  For pure ASCII input the result is identical to strscpy().
+ *
+ *  This matters for names that come from a device: usb_string() converts the
+ *  UTF-16LE string descriptors into UTF-8, so a product name copied into one
+ *  of the fixed-size card name fields can otherwise end in a partial sequence,
+ *  which user space has no way to repair.
+ *
+ *  Return: the length of the copied string, excluding the terminating NUL.
+ */
+size_t snd_utf8_strscpy(char *dst, const char *src, size_t size)
+{
+	size_t len, tail;
+
+	if (!size)
+		return 0;
+
+	len = strnlen(src, size);
+	if (len < size) {
+		memcpy(dst, src, len);
+		dst[len] = '\0';
+		return len;
+	}
+
+	/* @src does not fit.  strscpy() would cut at @size - 1, which may land
+	 * inside a multi-byte sequence, so back up to the start of that
+	 * sequence instead.  A sequence is at most four bytes, so a longer run
+	 * of continuation bytes means @src is not UTF-8 after all and the plain
+	 * cut stands.
+	 */
+	len = size - 1;
+	for (tail = 0; tail < 4 && len; tail++) {
+		if (((unsigned char)src[len] & 0xc0) != 0x80)
+			break;
+		len--;
+	}
+	if (tail >= 4)
+		len = size - 1;
+
+	memcpy(dst, src, len);
+	dst[len] = '\0';
+	return len;
+}
+EXPORT_SYMBOL_GPL(snd_utf8_strscpy);
+
 static ssize_t id_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
diff --git a/sound/core/sound_kunit.c b/sound/core/sound_kunit.c
index 0376112cc6..d462462543 100644
--- a/sound/core/sound_kunit.c
+++ b/sound/core/sound_kunit.c
@@ -299,6 +299,63 @@ static void test_card_add_component(struct kunit *test)
 	KUNIT_ASSERT_STREQ(test, card->components, TEST_FIRST_COMPONENT " " TEST_SECOND_COMPONENT);
 }
 
+/* snd_utf8_strscpy() has to behave exactly like strscpy() for ASCII and only
+ * differ where a multi-byte sequence would be cut.  The escapes keep this file
+ * plain ASCII; \x escapes are greedy, hence the split literals.
+ */
+#define STRSCPY_UMLAUT		"\xc3\xa4"	/* 2 bytes */
+#define STRSCPY_HIRAGANA	"\xe3\x81\x82"	/* 3 bytes */
+
+static void test_utf8_strscpy(struct kunit *test)
+{
+	char dst[8], ref[8];
+	size_t len;
+
+	/* fits: same result as strscpy(), NUL terminated */
+	len = snd_utf8_strscpy(dst, "abc", sizeof(dst));
+	KUNIT_EXPECT_EQ(test, len, 3);
+	KUNIT_EXPECT_STREQ(test, dst, "abc");
+
+	/* exactly fills the buffer */
+	len = snd_utf8_strscpy(dst, "abcdefg", sizeof(dst));
+	KUNIT_EXPECT_EQ(test, len, sizeof(dst) - 1);
+	KUNIT_EXPECT_STREQ(test, dst, "abcdefg");
+
+	/* too long and pure ASCII: identical to strscpy() */
+	len = snd_utf8_strscpy(dst, "abcdefghij", sizeof(dst));
+	strscpy(ref, "abcdefghij", sizeof(ref));
+	KUNIT_EXPECT_EQ(test, len, sizeof(dst) - 1);
+	KUNIT_EXPECT_STREQ(test, dst, ref);
+
+	/* 2-byte characters: 7 bytes of room hold three of them plus one ASCII
+	 * byte, so the fourth has to be dropped whole rather than halved
+	 */
+	len = snd_utf8_strscpy(dst, STRSCPY_UMLAUT STRSCPY_UMLAUT
+				    STRSCPY_UMLAUT STRSCPY_UMLAUT,
+			       sizeof(dst));
+	KUNIT_EXPECT_EQ(test, len, 6);
+	KUNIT_EXPECT_STREQ(test, dst, STRSCPY_UMLAUT STRSCPY_UMLAUT
+				      STRSCPY_UMLAUT);
+
+	/* 3-byte characters: two fit, the third would be cut */
+	len = snd_utf8_strscpy(dst, STRSCPY_HIRAGANA STRSCPY_HIRAGANA
+				    STRSCPY_HIRAGANA,
+			       sizeof(dst));
+	KUNIT_EXPECT_EQ(test, len, 6);
+	KUNIT_EXPECT_STREQ(test, dst, STRSCPY_HIRAGANA STRSCPY_HIRAGANA);
+
+	/* not UTF-8 at all: a run of continuation bytes longer than any
+	 * sequence, so the plain strscpy() cut stands rather than eating the
+	 * whole string
+	 */
+	len = snd_utf8_strscpy(dst, "\x80\x80\x80\x80\x80\x80\x80\x80\x80",
+			       sizeof(dst));
+	KUNIT_EXPECT_EQ(test, len, sizeof(dst) - 1);
+
+	/* zero-sized destination must not be written to */
+	KUNIT_EXPECT_EQ(test, snd_utf8_strscpy(dst, "abc", 0), 0);
+}
+
 static struct kunit_case sound_utils_cases[] = {
 	KUNIT_CASE(test_phys_format_size),
 	KUNIT_CASE(test_format_width),
@@ -310,6 +367,7 @@ static struct kunit_case sound_utils_cases[] = {
 	KUNIT_CASE(test_card_set_id),
 	KUNIT_CASE(test_pcm_format_name),
 	KUNIT_CASE(test_card_add_component),
+	KUNIT_CASE(test_utf8_strscpy),
 	{},
 };
 
diff --git a/sound/usb/card.c b/sound/usb/card.c
index 9307da95ef..a83c51a3d5 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -630,9 +630,12 @@ static void usb_audio_make_shortname(struct usb_device *dev,
 		return;
 	}
 
-	/* retrieve the device string as shortname */
+	/* retrieve the device string as shortname.  usb_string() hands back
+	 * UTF-8, so cut at a character boundary rather than at a byte one.
+	 */
 	if (dev->product && *dev->product) {
-		strscpy(card->shortname, dev->product);
+		snd_utf8_strscpy(card->shortname, dev->product,
+				 sizeof(card->shortname));
 	} else {
 		/* no name available from anywhere, so use ID */
 		scnprintf(card->shortname, sizeof(card->shortname),
-- 
2.43.0

[RFC PATCH 4/8] ALSA: core: carry the untruncated form of the card names

[Luca Rodenhäuser]

card->shortname, card->longname and card->mixername are fixed-size fields,
32 and 80 bytes, and a name that does not fit is cut.  The previous patch
stops that cut from landing inside a character, but it cannot make a name
fit: 32 bytes is eight CJK characters, so a cleanly cut name is still a cut
name, and user space has no way to recover what was dropped.

card->components already solves the same problem the other way round: the
card holds the whole string, and snd_ctl_card_info::components is a cut
copy of it, with the full one available through SNDRV_CTL_IOCTL_CARD_BYTES.
Do the same for the three names.

card->full_shortname and friends hold the whole name, allocated on demand,
and stay NULL when the name fit into its fixed field.  That is the point of
the design: "set" means "the fixed field is not the whole story", so there
is one name per card in two lengths, and never a pair that could disagree
or that a driver has to keep in sync.  Nothing user space sees today
changes, since the fixed fields keep exactly what they hold now.

Card names are UTF-8, which on USB they have been for decades, so the
kernel repairs what would break printing them: a malformed sequence becomes
U+FFFD, one per bad byte as recommended for a maximal subpart in Unicode
TR#36, and a code point that would break a line becomes a space.  The
second one matters because the kernel prints these names into
/proc/asound/cards, which is line based.  Rejecting a name instead would
achieve nothing: the fixed field would still hold it, and readers are told
to use that whenever the untruncated one is absent.

Nothing else is inspected.  Invisible and bidirectional format characters
are a real spoofing vector, but catching them needs the Unicode character
properties, which would mean a table in the sound core that goes stale with
every Unicode release - the same dependency avoided by leaving NFC to
drivers.  A partial range list is worse than none, because it invites user
space to skip its own filtering.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 include/sound/core.h |  27 +++++
 sound/core/init.c    | 272 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 299 insertions(+)

diff --git a/include/sound/core.h b/include/sound/core.h
index 36bb95044a..c00f091ea1 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -96,6 +96,16 @@ static inline void snd_refcount_get(struct snd_refcount *ref)
 void snd_refcount_put(struct snd_refcount *ref);
 void snd_refcount_sync(struct snd_refcount *ref);
 
+/*
+ * Maximum size in bytes, excluding the terminating NUL, of the untruncated
+ * card name fields.  Each limit is four times the size of the corresponding
+ * fixed field, so the same number of characters always fits, whatever they
+ * encode to: card names are UTF-8, and a character takes up to four bytes.
+ */
+#define SNDRV_CARD_FULL_SHORTNAME_MAX	128
+#define SNDRV_CARD_FULL_LONGNAME_MAX	320
+#define SNDRV_CARD_FULL_MIXERNAME_MAX	320
+
 /* main structure for soundcard */
 
 struct snd_card {
@@ -108,6 +118,20 @@ struct snd_card {
 	char longname[80];		/* name of this soundcard */
 	char irq_descr[32];		/* Interrupt description */
 	char mixername[80];		/* mixer name */
+	/*
+	 * The untruncated form of the three name fields above, like
+	 * card->components is the untruncated form of what
+	 * snd_ctl_card_info::components holds.  Each is NULL unless the name
+	 * did not fit into its fixed-size field, so a reader that finds NULL
+	 * already has the whole name in the field above.
+	 *
+	 * Set them only via snd_card_set_full_*(), and only before
+	 * snd_card_register().  After that they are read under
+	 * snd_ioctl_rwsem.
+	 */
+	char *full_shortname;		/* UTF-8 short name, may be NULL */
+	char *full_longname;		/* UTF-8 long name, may be NULL */
+	char *full_mixername;		/* UTF-8 mixer name, may be NULL */
 	char *components;		/* card components, space-delimited */
 	unsigned int components_alloc_size;	/* current allocation size of components */
 	struct module *module;		/* top-level module */
@@ -317,6 +341,9 @@ void snd_card_free_when_closed(struct snd_card *card);
 int snd_card_free_on_error(struct device *dev, int ret);
 void snd_card_set_id(struct snd_card *card, const char *id);
 size_t snd_utf8_strscpy(char *dst, const char *src, size_t size);
+int snd_card_set_full_shortname(struct snd_card *card, const char *name);
+int snd_card_set_full_longname(struct snd_card *card, const char *name);
+int snd_card_set_full_mixername(struct snd_card *card, const char *name);
 int snd_card_register(struct snd_card *card);
 int snd_card_info_init(void);
 int snd_card_add_dev_attr(struct snd_card *card,
diff --git a/sound/core/init.c b/sound/core/init.c
index 67b79b1826..cabe4da212 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -16,6 +16,7 @@
 #include <linux/debugfs.h>
 #include <linux/completion.h>
 #include <linux/interrupt.h>
+#include <linux/err.h>
 
 #include <sound/core.h>
 #include <sound/control.h>
@@ -146,6 +147,7 @@ static int snd_card_init(struct snd_card *card, struct device *parent,
 			 int idx, const char *xid, struct module *module,
 			 size_t extra_size);
 static int snd_card_do_free(struct snd_card *card);
+static void snd_card_free_full_names(struct snd_card *card);
 static const struct attribute_group card_dev_attr_group;
 
 static void release_card_device(struct device *dev)
@@ -594,6 +596,7 @@ static int snd_card_do_free(struct snd_card *card)
 	kfree(card->components);
 	card->components = NULL;
 	card->components_alloc_size = 0;
+	snd_card_free_full_names(card);
 	if (card->private_free)
 		card->private_free(card);
 #ifdef CONFIG_SND_CTL_DEBUG
@@ -841,6 +844,275 @@ size_t snd_utf8_strscpy(char *dst, const char *src, size_t size)
 }
 EXPORT_SYMBOL_GPL(snd_utf8_strscpy);
 
+/*
+ * Untruncated card names
+ *
+ * card->shortname, card->longname and card->mixername are fixed-size fields,
+ * and a name that does not fit is cut.  card->full_shortname and friends hold
+ * the whole name in that case, exactly as card->components holds the whole
+ * components string of which snd_ctl_card_info::components is a cut copy.
+ * User space reads them through SNDRV_CTL_IOCTL_CARD_BYTES.
+ *
+ * A field stays NULL when the name fits, so "set" means "the fixed field is
+ * not the whole story".  Nothing has to be kept in sync: there is one name,
+ * in two lengths.
+ *
+ * Card names are UTF-8.  The kernel prints them into procfs and hands them to
+ * user space, so it repairs what would break that: a malformed sequence
+ * becomes U+FFFD, and a code point that would break a line becomes a space.
+ * Rejecting a name instead would achieve nothing, since the fixed field still
+ * holds it and readers are told to use that when the full one is absent.
+ *
+ * Beyond that the kernel does not inspect names.  NFC, homoglyphs and the
+ * invisible and bidirectional format characters need the Unicode character
+ * properties, which would go stale in the sound core; user space has them.
+ * A name is a display string and never an identifier: card->id stays the
+ * unique handle.
+ */
+
+/* ASCII whitespace only; isspace() also matches 0xa0, which would eat UTF-8
+ * continuation bytes
+ */
+static bool utf8_is_ascii_space(unsigned char c)
+{
+	return c == ' ' || (c >= '\t' && c <= '\r');
+}
+
+/* Decode the UTF-8 sequence at @s, which holds @len readable bytes.
+ * Stores the code point in @cp and returns its length in bytes, or 0 if @s
+ * doesn't start a well-formed sequence.
+ */
+static size_t utf8_decode(const unsigned char *s, size_t len, u32 *cp)
+{
+	static const u32 min_cp[5] = { 0, 0, 0x80, 0x800, 0x10000 };
+	size_t i, n;
+	u32 val;
+
+	if (s[0] < 0x80) {
+		*cp = s[0];
+		return 1;
+	} else if ((s[0] & 0xe0) == 0xc0) {
+		n = 2;
+		val = s[0] & 0x1f;
+	} else if ((s[0] & 0xf0) == 0xe0) {
+		n = 3;
+		val = s[0] & 0x0f;
+	} else if ((s[0] & 0xf8) == 0xf0) {
+		n = 4;
+		val = s[0] & 0x07;
+	} else {
+		return 0;	/* stray continuation byte or 5/6-byte form */
+	}
+
+	if (len < n)
+		return 0;
+	for (i = 1; i < n; i++) {
+		if ((s[i] & 0xc0) != 0x80)
+			return 0;
+		val = (val << 6) | (s[i] & 0x3f);
+	}
+
+	/* overlong encoding, surrogate half or beyond the Unicode range */
+	if (val < min_cp[n] || val > 0x10ffff || (val >= 0xd800 && val <= 0xdfff))
+		return 0;
+
+	*cp = val;
+	return n;
+}
+
+/* U+FFFD REPLACEMENT CHARACTER */
+#define UTF8_REPLACEMENT	"\xef\xbf\xbd"
+
+/* Code points that would break the structure the name is printed into: sysfs
+ * attributes and /proc/asound/cards are line based, and a terminal acts on
+ * control characters.  They are replaced by a space rather than rejected.
+ *
+ * Nothing else is inspected.  Invisible, bidirectional and deprecated format
+ * characters are a real spoofing vector, but catching them needs the Unicode
+ * character properties, which would mean a table in the sound core that goes
+ * stale with every Unicode release.  A short list of ranges only looks like a
+ * defence; user space has the tables and has to do that part.
+ */
+static bool utf8_needs_space(u32 cp)
+{
+	if (cp < 0x20 || cp == 0x7f)		/* C0 controls and DEL */
+		return true;
+	if (cp >= 0x80 && cp <= 0x9f)		/* C1 controls */
+		return true;
+	if (cp == 0x2028 || cp == 0x2029)	/* line/paragraph separator */
+		return true;
+	return false;
+}
+
+/* Sanitise @src into a UTF-8 string of at most @max bytes.
+ *
+ * A malformed sequence becomes U+FFFD, one per bad byte, as recommended for
+ * the maximal subpart of an ill-formed subsequence in Unicode TR#36.  Code
+ * points that would break a line become a space.  The result is therefore
+ * always well-formed and always printable on one line, which means a caller
+ * never has to fall back to the ASCII field because a name was hostile.
+ *
+ * Surrounding whitespace is dropped and the result is cut at a character
+ * boundary, never inside a sequence.  Returns NULL when nothing usable is
+ * left, which the caller treats as "unset".
+ */
+static char *snd_card_name_dup(const char *src, size_t max)
+{
+	const unsigned char *s = (const unsigned char *)src;
+	size_t avail = strlen(src);
+	size_t in = 0, out = 0, start = 0;
+	char *buf, *shrunk;
+	size_t n;
+	u32 cp;
+
+	buf = kmalloc(max + 1, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	while (in < avail) {
+		n = utf8_decode(s + in, avail - in, &cp);
+		if (!n) {
+			if (out + sizeof(UTF8_REPLACEMENT) - 1 > max)
+				break;
+			memcpy(buf + out, UTF8_REPLACEMENT,
+			       sizeof(UTF8_REPLACEMENT) - 1);
+			out += sizeof(UTF8_REPLACEMENT) - 1;
+			in++;
+			continue;
+		}
+		if (utf8_needs_space(cp)) {
+			if (out + 1 > max)
+				break;
+			buf[out++] = ' ';
+			in += n;
+			continue;
+		}
+		if (out + n > max)
+			break;		/* cut at a character boundary */
+		memcpy(buf + out, s + in, n);
+		out += n;
+		in += n;
+	}
+
+	/* drop surrounding whitespace, including what the replacing above may
+	 * have produced at either end
+	 */
+	while (start < out && utf8_is_ascii_space(buf[start]))
+		start++;
+	while (out > start && utf8_is_ascii_space(buf[out - 1]))
+		out--;
+	if (out == start) {
+		kfree(buf);
+		return NULL;
+	}
+	out -= start;
+	memmove(buf, buf + start, out);
+	buf[out] = '\0';
+
+	shrunk = krealloc(buf, out + 1, GFP_KERNEL);
+	return shrunk ? : buf;	/* shrinking does not fail, but do not rely on it */
+}
+
+static int snd_card_set_full_name(struct snd_card *card, char **dst,
+				  const char *ascii, const char *name,
+				  size_t max)
+{
+	char *copy = NULL, *old;
+
+	if (name) {
+		copy = snd_card_name_dup(name, max);
+		if (IS_ERR(copy))
+			return PTR_ERR(copy);
+		/* Nothing gained over the ASCII field: leave the field unset so
+		 * the documented fallback does the same job without a second
+		 * allocation.  This keeps "set" meaning "carries more".
+		 */
+		if (copy && !strcmp(copy, ascii)) {
+			kfree(copy);
+			copy = NULL;
+		}
+	}
+
+	scoped_guard(rwsem_write, &snd_ioctl_rwsem) {
+		old = *dst;
+		*dst = copy;
+	}
+	kfree(old);
+	return 0;
+}
+
+static void snd_card_free_full_names(struct snd_card *card)
+{
+	kfree(card->full_shortname);
+	card->full_shortname = NULL;
+	kfree(card->full_longname);
+	card->full_longname = NULL;
+	kfree(card->full_mixername);
+	card->full_mixername = NULL;
+}
+
+/**
+ *  snd_card_set_full_shortname - set the UTF-8 short name of the card
+ *  @card: soundcard structure
+ *  @name: UTF-8 name, or NULL to clear the field
+ *
+ *  Offers the untruncated form of card->shortname.  Set card->shortname first,
+ *  with snd_utf8_strscpy(): if the sanitised name comes out identical to it,
+ *  the name fit and the field is left unset, so "set" always means "the fixed
+ *  field is cut".
+ *
+ *  @name is sanitised, never rejected: malformed sequences become U+FFFD and
+ *  line breaking code points become a space.  Surrounding whitespace is
+ *  stripped and a name longer than SNDRV_CARD_FULL_SHORTNAME_MAX bytes is cut
+ *  at a character boundary.  The caller should normalise @name to NFC
+ *  beforehand; the kernel cannot.  Call this before snd_card_register().
+ *
+ *  Return: Zero on success, -ENOMEM on allocation failure.
+ */
+int snd_card_set_full_shortname(struct snd_card *card, const char *name)
+{
+	return snd_card_set_full_name(card, &card->full_shortname,
+				      card->shortname, name,
+				      SNDRV_CARD_FULL_SHORTNAME_MAX);
+}
+EXPORT_SYMBOL_GPL(snd_card_set_full_shortname);
+
+/**
+ *  snd_card_set_full_longname - set the UTF-8 long name of the card
+ *  @card: soundcard structure
+ *  @name: UTF-8 name, or NULL to clear the field
+ *
+ *  Offers the untruncated form of card->longname; see
+ *  snd_card_set_full_shortname() for the rules that apply.
+ *
+ *  Return: Zero on success, -ENOMEM on allocation failure.
+ */
+int snd_card_set_full_longname(struct snd_card *card, const char *name)
+{
+	return snd_card_set_full_name(card, &card->full_longname,
+				      card->longname, name,
+				      SNDRV_CARD_FULL_LONGNAME_MAX);
+}
+EXPORT_SYMBOL_GPL(snd_card_set_full_longname);
+
+/**
+ *  snd_card_set_full_mixername - set the UTF-8 mixer name of the card
+ *  @card: soundcard structure
+ *  @name: UTF-8 name, or NULL to clear the field
+ *
+ *  Offers the untruncated form of card->mixername; see
+ *  snd_card_set_full_shortname() for the rules that apply.
+ *
+ *  Return: Zero on success, -ENOMEM on allocation failure.
+ */
+int snd_card_set_full_mixername(struct snd_card *card, const char *name)
+{
+	return snd_card_set_full_name(card, &card->full_mixername,
+				      card->mixername, name,
+				      SNDRV_CARD_FULL_MIXERNAME_MAX);
+}
+EXPORT_SYMBOL_GPL(snd_card_set_full_mixername);
+
 static ssize_t id_show(struct device *dev,
 		       struct device_attribute *attr, char *buf)
 {
-- 
2.43.0

[RFC PATCH 5/8] ALSA: control: return the untruncated card names via CARD_BYTES

[Luca Rodenhäuser]

struct snd_ctl_card_info is fixed-size and cannot grow, so the untruncated
names need another way out.  SNDRV_CTL_IOCTL_CARD_BYTES already carries one
untruncated card string, with a query-then-read protocol and a __u64 user
pointer that needs no compat handling, so add three types to it rather than
a new ioctl: SND_CTL_CARD_BTYPE_NAME, _LONGNAME and _MIXERNAME, next to the
existing _COMPONENTS.  No existing structure or ioctl changes, so old
binaries are untouched.

A name that fits into its fixed field is returned as an empty string rather
than repeated, which tells the reader that snd_ctl_card_info already has
the whole name.  An older kernel answers -EINVAL for the unknown type,
which means the same thing, so a reader needs no version check; PVERSION
reports 2.0.11 where the types exist.

The header says what the kernel guarantees about a name - well-formed
UTF-8, nothing that would break a line, never cut inside a sequence - and
at more length what it does not: no NFC, no homoglyph detection, and no
filtering of invisible or bidirectional format characters, with a pointer
at what user space has to do instead.  A promise that cannot be kept is
worse than no promise, because it invites readers to skip their own
filtering.

Pull the string lookup out into a helper so all four types share one output
path instead of repeating the copy-out four times.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 include/uapi/sound/asound.h | 30 +++++++++++++++++++++++++++++-
 sound/core/control.c        | 36 ++++++++++++++++++++++++++++--------
 2 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index c11da9656e..bebd1b1049 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -1058,7 +1058,7 @@ struct snd_timer_tread {
  *                                                                          *
  ****************************************************************************/
 
-#define SNDRV_CTL_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 10)
+#define SNDRV_CTL_VERSION		SNDRV_PROTOCOL_VERSION(2, 0, 11)
 
 struct snd_ctl_card_info {
 	int card;			/* card number */
@@ -1076,11 +1076,39 @@ struct snd_ctl_card_info {
  * Card components can exceed the fixed 128 bytes in snd_ctl_card_info.
  * Use SNDRV_CTL_IOCTL_CARD_BYTES with type SND_CTL_CARD_BTYPE_COMPONENTS
  * to retrieve the full string.
+ *
+ * SND_CTL_CARD_BTYPE_NAME, _LONGNAME and _MIXERNAME return the untruncated
+ * form of the name, longname and mixername fields of struct snd_ctl_card_info,
+ * in the same way SND_CTL_CARD_BTYPE_COMPONENTS returns the untruncated
+ * components string.  Available since SNDRV_CTL_VERSION 2.0.11.
+ *
+ * A name that fits into its fixed field is not repeated here: an empty result
+ * means the field in struct snd_ctl_card_info already holds the whole name.
+ * An older kernel answers -EINVAL for the unknown type, which means the same.
+ * So there is one name per card, available in two lengths, and never two names
+ * that could disagree.
+ *
+ * Card names are UTF-8.  The kernel guarantees that what it returns here is
+ * well-formed UTF-8 carrying no code point that would break a line (C0 and C1
+ * controls, U+2028, U+2029), and that it is never cut inside a multi-byte
+ * sequence.  The fixed fields are cut at a character boundary too.
+ *
+ * Nothing beyond that is guaranteed, deliberately.  Invisible, bidirectional
+ * and deprecated format characters pass through, NFC is only a recommendation
+ * to drivers, and homoglyphs cannot be detected at all: all of that needs the
+ * Unicode character properties, which belong in user space and not in the
+ * sound core, and a range list in the kernel would go stale with every
+ * Unicode release.  So a name is a display string and never a security
+ * boundary: isolate it for display (U+2066/U+2069), filter
+ * Default_Ignorable_Code_Point, and match cards on id, driver or components.
  */
 
 /* Type values for struct snd_ctl_card_bytes::type */
 enum {
 	SND_CTL_CARD_BTYPE_COMPONENTS	= 1,	/* full card components string */
+	SND_CTL_CARD_BTYPE_NAME		= 2,	/* full short name */
+	SND_CTL_CARD_BTYPE_LONGNAME	= 3,	/* full long name */
+	SND_CTL_CARD_BTYPE_MIXERNAME	= 4,	/* full mixer name */
 };
 
 struct snd_ctl_card_bytes {
diff --git a/sound/core/control.c b/sound/core/control.c
index e404cb55a5..cfe451f0f4 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -14,6 +14,7 @@
 #include <linux/mm.h>
 #include <linux/math64.h>
 #include <linux/sched/signal.h>
+#include <linux/err.h>
 #include <linux/cleanup.h>
 #include <sound/core.h>
 #include <sound/minors.h>
@@ -901,6 +902,26 @@ static int snd_ctl_card_info(struct snd_card *card, struct snd_ctl_file * ctl,
 	return 0;
 }
 
+/* Pick the card string a SND_CTL_CARD_BTYPE_* value refers to.
+ * Returns NULL for an unset field, which is handed out as an empty string.
+ * Must be called with snd_ioctl_rwsem held.
+ */
+static const char *snd_ctl_card_btype_str(struct snd_card *card, __u32 type)
+{
+	switch (type) {
+	case SND_CTL_CARD_BTYPE_COMPONENTS:
+		return card->components;
+	case SND_CTL_CARD_BTYPE_NAME:
+		return card->full_shortname;
+	case SND_CTL_CARD_BTYPE_LONGNAME:
+		return card->full_longname;
+	case SND_CTL_CARD_BTYPE_MIXERNAME:
+		return card->full_mixername;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+}
+
 static int snd_ctl_card_bytes(struct snd_card *card,
 			      struct snd_ctl_card_bytes *info,
 			      unsigned int __user *data_len_out)
@@ -909,22 +930,21 @@ static int snd_ctl_card_bytes(struct snd_card *card,
 	bool too_small = false;
 	unsigned int data_len;
 
-	if (info->type != SND_CTL_CARD_BTYPE_COMPONENTS)
-		return -EINVAL;
-
 	scoped_guard(rwsem_read, &snd_ioctl_rwsem) {
-		const char *components = card->components;
+		const char *str = snd_ctl_card_btype_str(card, info->type);
 
-		if (!components)
-			components = "";
+		if (IS_ERR(str))
+			return PTR_ERR(str);
+		if (!str)
+			str = "";
 
-		data_len = strlen(components) + 1;
+		data_len = strlen(str) + 1;
 
 		if (info->data && info->data_allocated != 0) {
 			if (info->data_allocated < data_len) {
 				too_small = true;
 			} else {
-				copy = kmemdup(components, data_len, GFP_KERNEL);
+				copy = kmemdup(str, data_len, GFP_KERNEL);
 				if (!copy)
 					return -ENOMEM;
 			}
-- 
2.43.0

[RFC PATCH 6/8] ALSA: core: add KUnit coverage for the card names

[Luca Rodenhäuser]

Cover what the setters promise: plain ASCII and multi-byte names survive
byte for byte, a field starts out and can be cleared back to NULL,
whitespace is stripped, a whitespace-only name counts as unset, and a name
that fits into the fixed field next to it is not stored at all.

The sanitising cases carry the weight, since they are what keeps malformed
input from reaching user space: truncated sequences, stray continuation
bytes, overlong encodings, surrogate halves, code points past U+10FFFF, C0
and C1 controls and an ANSI escape, each with the exact output expected.

test_card_full_names_passthrough pins the opposite: bidi overrides,
zero-width and tag characters and a non-character come back unchanged.
That is deliberate, and a test is the only way to keep it from looking like
an oversight later.

Truncation is checked with a 2-byte and a 3-byte character, so the case
where the limit does not divide evenly is covered, and with input that is
all bad bytes, where each one grows from one byte to three and the limit
has to hold on the output rather than the input.

test_card_id_stays_ascii is a regression test for the card id fix earlier
in this series; without it, card->id[0] comes out as 0xc3.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 sound/core/sound_kunit.c | 284 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 280 insertions(+), 4 deletions(-)

diff --git a/sound/core/sound_kunit.c b/sound/core/sound_kunit.c
index d462462543..26554da484 100644
--- a/sound/core/sound_kunit.c
+++ b/sound/core/sound_kunit.c
@@ -5,6 +5,7 @@
  */
 
 #include <kunit/test.h>
+#include <linux/ctype.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 
@@ -300,8 +301,7 @@ static void test_card_add_component(struct kunit *test)
 }
 
 /* snd_utf8_strscpy() has to behave exactly like strscpy() for ASCII and only
- * differ where a multi-byte sequence would be cut.  The escapes keep this file
- * plain ASCII; \x escapes are greedy, hence the split literals.
+ * differ where a multi-byte sequence would be cut.
  */
 #define STRSCPY_UMLAUT		"\xc3\xa4"	/* 2 bytes */
 #define STRSCPY_HIRAGANA	"\xe3\x81\x82"	/* 3 bytes */
@@ -327,8 +327,8 @@ static void test_utf8_strscpy(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, len, sizeof(dst) - 1);
 	KUNIT_EXPECT_STREQ(test, dst, ref);
 
-	/* 2-byte characters: 7 bytes of room hold three of them plus one ASCII
-	 * byte, so the fourth has to be dropped whole rather than halved
+	/* 2-byte characters: three fit in seven bytes, the fourth has to be
+	 * dropped whole rather than halved
 	 */
 	len = snd_utf8_strscpy(dst, STRSCPY_UMLAUT STRSCPY_UMLAUT
 				    STRSCPY_UMLAUT STRSCPY_UMLAUT,
@@ -356,6 +356,275 @@ static void test_utf8_strscpy(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, snd_utf8_strscpy(dst, "abc", 0), 0);
 }
 
+/* The UTF-8 test names are spelled out as escapes on purpose: the bytes are
+ * what is under test, and the file itself stays plain ASCII.  Literals are
+ * split where a hex escape is followed by a hex digit, since \x escapes are
+ * greedy: "\xe2\x80\xaedef" would parse as \xe2 \x80 \xaed.
+ */
+#define UTF8_LATIN_NAME		"M\xc3\xbcller Audio"		/* Mueller with umlaut */
+#define UTF8_CJK_NAME		"\xe3\x81\x82\xe9\x9f\xb3"	/* hiragana a + kanji */
+#define UTF8_ASTRAL_NAME	"\xf0\x9f\x8e\xb5 Audio"	/* musical note + text */
+#define UTF8_UNTRIMMED_NAME	" \t" UTF8_LATIN_NAME " \n"
+#define UTF8_ONLY_SPACES	"  \t\r\n "
+#define UTF8_REPLACEMENT	"\xef\xbf\xbd"			/* U+FFFD */
+
+#define UTF8_UMLAUT		"\xc3\xa4"			/* 2 bytes */
+#define UTF8_HIRAGANA		"\xe3\x81\x82"			/* 3 bytes */
+
+/* Malformed input.  None of it is rejected any more; each bad byte becomes one
+ * U+FFFD, so that a caller is never pushed back to the unfiltered ASCII field.
+ */
+static const struct {
+	const char *in;
+	const char *out;
+} sanitised_names[] = {
+	/* truncated 2-byte start */
+	{ "abc\xc3",			"abc" UTF8_REPLACEMENT },
+	/* stray continuation byte */
+	{ "abc\x80",			"abc" UTF8_REPLACEMENT },
+	/* overlong encoding of '/': both bytes are bad */
+	{ "abc\xc0\xaf",		"abc" UTF8_REPLACEMENT UTF8_REPLACEMENT },
+	/* U+D800, a surrogate half: three bad bytes */
+	{ "abc\xed\xa0\x80",		"abc" UTF8_REPLACEMENT UTF8_REPLACEMENT
+					UTF8_REPLACEMENT },
+	/* beyond U+10FFFF */
+	{ "abc\xf5\x80\x80\x80",	"abc" UTF8_REPLACEMENT UTF8_REPLACEMENT
+					UTF8_REPLACEMENT UTF8_REPLACEMENT },
+	/* broken continuation: the 'A' after it is valid and survives */
+	{ "abc\xe3\x81" "A",		"abc" UTF8_REPLACEMENT UTF8_REPLACEMENT "A" },
+	/* C0 control in the middle becomes a space */
+	{ "abc\ndef",			"abc def" },
+	/* U+0085 NEL, a C1 control */
+	{ "abc\xc2\x85" "def",		"abc def" },
+	/* ANSI escape: the ESC is what makes it dangerous */
+	{ "abc\x1b[31mdef",		"abc [31mdef" },
+	/* U+2028 LINE SEPARATOR */
+	{ "abc\xe2\x80\xa8" "def",	"abc def" },
+};
+
+/* Code points the kernel deliberately does NOT touch.  They are a real
+ * spoofing vector, but catching them needs the Unicode character properties,
+ * which live in user space.  This test pins the documented behaviour so that
+ * nobody mistakes it for an oversight.
+ */
+static const char * const passthrough_names[] = {
+	"Audio\xd8\x9c" "Device",	/* U+061C ARABIC LETTER MARK */
+	"Audio\xe2\x80\xae" "Device",	/* U+202E RIGHT-TO-LEFT OVERRIDE */
+	"Audio\xe2\x81\xaa" "Device",	/* U+206A INHIBIT SYMMETRIC SWAPPING */
+	"Au\xc2\xad" "dio",		/* U+00AD SOFT HYPHEN */
+	"Audio\xef\xb8\x8f",		/* U+FE0F VARIATION SELECTOR-16 */
+	"Audio\xf3\xa0\x80\x81",	/* U+E0001 LANGUAGE TAG */
+	"Audio\xef\xbf\xbf",		/* U+FFFF non-character */
+};
+
+/* Build @repeat copies of @seq, NUL terminated. */
+static char *utf8_repeat(struct kunit *test, const char *seq, size_t repeat)
+{
+	size_t seq_len = strlen(seq);
+	char *buf;
+	size_t i;
+
+	buf = kunit_kzalloc(test, seq_len * repeat + 1, GFP_KERNEL);
+	if (!buf)
+		return NULL;
+	for (i = 0; i < repeat; i++)
+		memcpy(buf + i * seq_len, seq, seq_len);
+	return buf;
+}
+
+static void test_card_full_names(struct kunit *test)
+{
+	struct snd_card *card = kunit_kzalloc(test, sizeof(*card), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, card);
+
+	/* an unset field stays NULL, which means "fall back to ASCII" */
+	KUNIT_EXPECT_NULL(test, card->full_shortname);
+	KUNIT_EXPECT_NULL(test, card->full_longname);
+	KUNIT_EXPECT_NULL(test, card->full_mixername);
+
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, UTF8_LATIN_NAME));
+	KUNIT_EXPECT_STREQ(test, card->full_shortname, UTF8_LATIN_NAME);
+
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_longname(card, UTF8_CJK_NAME));
+	KUNIT_EXPECT_STREQ(test, card->full_longname, UTF8_CJK_NAME);
+
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_mixername(card, UTF8_ASTRAL_NAME));
+	KUNIT_EXPECT_STREQ(test, card->full_mixername, UTF8_ASTRAL_NAME);
+
+	/* setting it again replaces the previous value */
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, UTF8_CJK_NAME));
+	KUNIT_EXPECT_STREQ(test, card->full_shortname, UTF8_CJK_NAME);
+
+	/* NULL clears the field back to "unset" */
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, NULL));
+	KUNIT_EXPECT_NULL(test, card->full_shortname);
+
+	snd_card_set_full_longname(card, NULL);
+	snd_card_set_full_mixername(card, NULL);
+}
+
+/* A UTF-8 field is only worth an allocation when it carries more than the
+ * ASCII field next to it; otherwise the documented fallback says the same.
+ */
+static void test_card_utf8_names_same_as_ascii(struct kunit *test)
+{
+	struct snd_card *card = kunit_kzalloc(test, sizeof(*card), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, card);
+
+	strscpy(card->shortname, VALID_NAME);
+
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, VALID_NAME));
+	KUNIT_EXPECT_NULL(test, card->full_shortname);
+
+	/* the same name with padding sanitises to the ASCII one, so still unset */
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, " " VALID_NAME "\t"));
+	KUNIT_EXPECT_NULL(test, card->full_shortname);
+
+	/* one that differs is kept */
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, UTF8_LATIN_NAME));
+	KUNIT_EXPECT_STREQ(test, card->full_shortname, UTF8_LATIN_NAME);
+
+	snd_card_set_full_shortname(card, NULL);
+}
+
+static void test_card_utf8_names_sanitised(struct kunit *test)
+{
+	struct snd_card *card = kunit_kzalloc(test, sizeof(*card), GFP_KERNEL);
+	size_t i;
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, card);
+
+	for (i = 0; i < ARRAY_SIZE(sanitised_names); i++) {
+		KUNIT_EXPECT_EQ_MSG(test, 0,
+				    snd_card_set_full_shortname(card, sanitised_names[i].in),
+				    "name %zu was rejected", i);
+		KUNIT_EXPECT_STREQ_MSG(test, card->full_shortname,
+				       sanitised_names[i].out,
+				       "name %zu came out wrong", i);
+	}
+
+	snd_card_set_full_shortname(card, NULL);
+}
+
+static void test_card_utf8_names_passthrough(struct kunit *test)
+{
+	struct snd_card *card = kunit_kzalloc(test, sizeof(*card), GFP_KERNEL);
+	size_t i;
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, card);
+
+	for (i = 0; i < ARRAY_SIZE(passthrough_names); i++) {
+		KUNIT_EXPECT_EQ(test, 0,
+				snd_card_set_full_shortname(card, passthrough_names[i]));
+		KUNIT_EXPECT_STREQ_MSG(test, card->full_shortname,
+				       passthrough_names[i],
+				       "name %zu was altered; the kernel does not filter these",
+				       i);
+	}
+
+	snd_card_set_full_shortname(card, NULL);
+}
+
+static void test_card_utf8_names_trimmed(struct kunit *test)
+{
+	struct snd_card *card = kunit_kzalloc(test, sizeof(*card), GFP_KERNEL);
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, card);
+
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, UTF8_UNTRIMMED_NAME));
+	KUNIT_EXPECT_STREQ(test, card->full_shortname, UTF8_LATIN_NAME);
+
+	/* a name that is only whitespace carries no information: treat it as
+	 * unset rather than storing an empty string
+	 */
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, UTF8_ONLY_SPACES));
+	KUNIT_EXPECT_NULL(test, card->full_shortname);
+
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, ""));
+	KUNIT_EXPECT_NULL(test, card->full_shortname);
+
+	/* control characters at the ends become spaces and are then trimmed */
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, "\x01" VALID_NAME "\x7f"));
+	KUNIT_EXPECT_STREQ(test, card->full_shortname, VALID_NAME);
+
+	snd_card_set_full_shortname(card, NULL);
+}
+
+static void test_card_utf8_names_truncated(struct kunit *test)
+{
+	struct snd_card *card = kunit_kzalloc(test, sizeof(*card), GFP_KERNEL);
+	size_t len;
+	char *name;
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, card);
+
+	/* 2-byte characters divide the limit evenly */
+	name = utf8_repeat(test, UTF8_UMLAUT, SNDRV_CARD_FULL_SHORTNAME_MAX);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, name);
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, name));
+	KUNIT_ASSERT_NOT_NULL(test, card->full_shortname);
+	KUNIT_EXPECT_EQ(test, strlen(card->full_shortname),
+			SNDRV_CARD_FULL_SHORTNAME_MAX);
+
+	/* 3-byte characters do not: the last whole character has to win, so the
+	 * result stays below the limit instead of ending inside a sequence
+	 */
+	name = utf8_repeat(test, UTF8_HIRAGANA, SNDRV_CARD_FULL_SHORTNAME_MAX);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, name);
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, name));
+	KUNIT_ASSERT_NOT_NULL(test, card->full_shortname);
+	len = strlen(card->full_shortname);
+	KUNIT_EXPECT_LE(test, len, (size_t)SNDRV_CARD_FULL_SHORTNAME_MAX);
+	KUNIT_EXPECT_EQ(test, len % strlen(UTF8_HIRAGANA), 0);
+
+	/* replacing a bad byte grows it from one byte to three, so the limit
+	 * has to be enforced on the output, not on the input
+	 */
+	name = utf8_repeat(test, "\x80", SNDRV_CARD_FULL_SHORTNAME_MAX);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, name);
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_shortname(card, name));
+	KUNIT_ASSERT_NOT_NULL(test, card->full_shortname);
+	len = strlen(card->full_shortname);
+	KUNIT_EXPECT_LE(test, len, (size_t)SNDRV_CARD_FULL_SHORTNAME_MAX);
+	KUNIT_EXPECT_EQ(test, len % strlen(UTF8_REPLACEMENT), 0);
+
+	/* the longname field has its own, larger limit */
+	name = utf8_repeat(test, UTF8_UMLAUT, SNDRV_CARD_FULL_LONGNAME_MAX);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, name);
+	KUNIT_EXPECT_EQ(test, 0, snd_card_set_full_longname(card, name));
+	KUNIT_ASSERT_NOT_NULL(test, card->full_longname);
+	KUNIT_EXPECT_EQ(test, strlen(card->full_longname),
+			SNDRV_CARD_FULL_LONGNAME_MAX);
+
+	snd_card_set_full_shortname(card, NULL);
+	snd_card_set_full_longname(card, NULL);
+}
+
+/* card->id is an identifier, not a display name: it has to stay alphanumeric
+ * ASCII even when the name it is derived from is not.  isalpha() alone does
+ * not ensure that, since the ctype table treats the Latin-1 high bytes as
+ * letters.
+ */
+static void test_card_id_stays_ascii(struct kunit *test)
+{
+	struct snd_card *card = kunit_kzalloc(test, sizeof(*card), GFP_KERNEL);
+	size_t i;
+
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, card);
+
+	/* the digit has to be the first ASCII-safe byte to reach the path that
+	 * copies the source byte unfiltered
+	 */
+	snd_card_set_id(card, "\xc3\x84" "3000");
+
+	for (i = 0; card->id[i]; i++)
+		KUNIT_EXPECT_TRUE_MSG(test, isascii(card->id[i]),
+				      "card->id[%zu] = 0x%02x is not ASCII",
+				      i, (unsigned char)card->id[i]);
+}
+
 static struct kunit_case sound_utils_cases[] = {
 	KUNIT_CASE(test_phys_format_size),
 	KUNIT_CASE(test_format_width),
@@ -368,6 +637,13 @@ static struct kunit_case sound_utils_cases[] = {
 	KUNIT_CASE(test_pcm_format_name),
 	KUNIT_CASE(test_card_add_component),
 	KUNIT_CASE(test_utf8_strscpy),
+	KUNIT_CASE(test_card_full_names),
+	KUNIT_CASE(test_card_utf8_names_same_as_ascii),
+	KUNIT_CASE(test_card_utf8_names_sanitised),
+	KUNIT_CASE(test_card_utf8_names_passthrough),
+	KUNIT_CASE(test_card_utf8_names_trimmed),
+	KUNIT_CASE(test_card_utf8_names_truncated),
+	KUNIT_CASE(test_card_id_stays_ascii),
 	{},
 };
 
-- 
2.43.0

[RFC PATCH 7/8] ALSA: usb-audio: offer the untruncated card names

[Luca Rodenhäuser]

usb_string() hands back UTF-8 converted from the UTF-16LE string
descriptors, so dev->product and dev->manufacturer are exactly the kind of
name that does not always fit into a 32- or 80-byte field.

Offer the untruncated form alongside.  The fixed fields keep their current
contents, so user space that reads them today sees no change; since the
core stores the untruncated form only when the name did not fit, a device
name that fits costs nothing here.

The long name is assembled from the same pieces as the fixed one, but from
the untruncated sources, so it does not inherit the 80-byte cut.  The speed
suffix moves into a helper shared by both, and a small trim helper takes
the place of strim(), which cannot be used on the const sources.

This is best-effort: it can only fail with -ENOMEM, and the fixed name is
already in place, which is what a reader falls back to.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 sound/usb/card.c | 103 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 80 insertions(+), 23 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index a83c51a3d5..7397febed9 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -627,15 +627,23 @@ static void usb_audio_make_shortname(struct usb_device *dev,
 		s = quirk->product_name;
 	if (s && *s) {
 		strscpy(card->shortname, s, sizeof(card->shortname));
+		/* The UTF-8 field is optional and only stored when it carries
+		 * more than the ASCII one above, so this is a no-op for a plain
+		 * ASCII name that fits.  It can only fail with -ENOMEM, and the
+		 * ASCII name is already in place, so ignore the result.
+		 */
+		snd_card_set_full_shortname(card, s);
 		return;
 	}
 
-	/* retrieve the device string as shortname.  usb_string() hands back
-	 * UTF-8, so cut at a character boundary rather than at a byte one.
+	/* retrieve the device string as shortname; USB string descriptors are
+	 * converted to UTF-8 by usb_string(), so dev->product may well carry
+	 * characters that only fit into the UTF-8 field
 	 */
 	if (dev->product && *dev->product) {
 		snd_utf8_strscpy(card->shortname, dev->product,
 				 sizeof(card->shortname));
+		snd_card_set_full_shortname(card, dev->product);
 	} else {
 		/* no name available from anywhere, so use ID */
 		scnprintf(card->shortname, sizeof(card->shortname),
@@ -647,6 +655,69 @@ static void usb_audio_make_shortname(struct usb_device *dev,
 	strim(card->shortname);
 }
 
+static const char *usb_audio_speed_string(struct usb_device *dev)
+{
+	switch (snd_usb_get_speed(dev)) {
+	case USB_SPEED_LOW:
+		return ", low speed";
+	case USB_SPEED_FULL:
+		return ", full speed";
+	case USB_SPEED_HIGH:
+		return ", high speed";
+	case USB_SPEED_SUPER:
+		return ", super speed";
+	case USB_SPEED_SUPER_PLUS:
+		return ", super speed plus";
+	default:
+		return "";
+	}
+}
+
+/* Skip leading and trailing spaces of @s without touching it, storing the
+ * remaining length in @len.  strim() cannot be used here because the source
+ * strings are const and must stay untruncated.
+ */
+static const char *usb_audio_trim(const char *s, int *len)
+{
+	int n;
+
+	if (!s) {
+		*len = 0;
+		return "";
+	}
+	while (*s == ' ' || (*s >= '\t' && *s <= '\r'))
+		s++;
+	n = strlen(s);
+	while (n && (s[n - 1] == ' ' || (s[n - 1] >= '\t' && s[n - 1] <= '\r')))
+		n--;
+	*len = n;
+	return s;
+}
+
+/* Assemble the UTF-8 long name from the same pieces as the ASCII one, but from
+ * the untruncated sources.
+ */
+static void usb_audio_make_utf8_longname(struct usb_device *dev,
+					 struct snd_card *card,
+					 const char *vendor)
+{
+	char *longname __free(kfree) = NULL;
+	char path[64];
+	int vendor_len;
+
+	/* the short name has been built already, prefer its UTF-8 form */
+	const char *product = card->full_shortname ? : card->shortname;
+
+	vendor = usb_audio_trim(vendor, &vendor_len);
+	usb_make_path(dev, path, sizeof(path));
+
+	longname = kasprintf(GFP_KERNEL, "%.*s%s%s at %s%s",
+			     vendor_len, vendor, vendor_len ? " " : "",
+			     product, path, usb_audio_speed_string(dev));
+	if (longname)
+		snd_card_set_full_longname(card, longname);
+}
+
 static void usb_audio_make_longname(struct usb_device *dev,
 				    struct snd_usb_audio *chip,
 				    const struct snd_usb_audio_quirk *quirk)
@@ -663,6 +734,7 @@ static void usb_audio_make_longname(struct usb_device *dev,
 		s = preset->profile_name;
 	if (s && *s) {
 		strscpy(card->longname, s, sizeof(card->longname));
+		snd_card_set_full_longname(card, s);
 		return;
 	}
 
@@ -670,11 +742,12 @@ static void usb_audio_make_longname(struct usb_device *dev,
 		s = preset->vendor_name;
 	else if (quirk && quirk->vendor_name)
 		s = quirk->vendor_name;
+	if (!(s && *s) && dev->manufacturer && *dev->manufacturer)
+		s = dev->manufacturer;
+
 	*card->longname = 0;
 	if (s && *s)
 		strscpy(card->longname, s);
-	else if (dev->manufacturer && *dev->manufacturer)
-		strscpy(card->longname, dev->manufacturer);
 
 	if (*card->longname) {
 		strim(card->longname);
@@ -689,25 +762,9 @@ static void usb_audio_make_longname(struct usb_device *dev,
 	if (len < sizeof(card->longname))
 		usb_make_path(dev, card->longname + len, sizeof(card->longname) - len);
 
-	switch (snd_usb_get_speed(dev)) {
-	case USB_SPEED_LOW:
-		strlcat(card->longname, ", low speed", sizeof(card->longname));
-		break;
-	case USB_SPEED_FULL:
-		strlcat(card->longname, ", full speed", sizeof(card->longname));
-		break;
-	case USB_SPEED_HIGH:
-		strlcat(card->longname, ", high speed", sizeof(card->longname));
-		break;
-	case USB_SPEED_SUPER:
-		strlcat(card->longname, ", super speed", sizeof(card->longname));
-		break;
-	case USB_SPEED_SUPER_PLUS:
-		strlcat(card->longname, ", super speed plus", sizeof(card->longname));
-		break;
-	default:
-		break;
-	}
+	strlcat(card->longname, usb_audio_speed_string(dev), sizeof(card->longname));
+
+	usb_audio_make_utf8_longname(dev, card, s);
 }
 
 static void snd_usb_init_quirk_flags(int idx, struct snd_usb_audio *chip)
-- 
2.43.0

[RFC PATCH 8/8] ALSA: Documentation: describe the card name fields

[Luca Rodenhäuser]

Write down that card names are UTF-8, that the fixed fields are cut at a
character boundary, and that the untruncated form is available through
SNDRV_CTL_IOCTL_CARD_BYTES the same way the components string is.

Say what the kernel guarantees and, at more length, what it does not: no
NFC normalisation, no homoglyph detection, and no filtering of invisible or
bidirectional format characters.  Say why - those need Unicode character
properties that would go stale in the kernel - and say what user space has
to do instead, down to isolating names for display and filtering
Default_Ignorable_Code_Point.

Explain why a name is repaired rather than rejected, since that is the
non-obvious part: the fixed field holds the name either way, and readers
are told to use it when the untruncated one is absent, so rejection would
protect nobody.

Spell out why id, driver and components are not part of this: they are
identifiers, and matching a card has to keep using them.

Assisted-by: LLM
Signed-off-by: Luca Rodenhäuser <otzelot2021@outlook.de>
---
 Documentation/sound/designs/card-names.rst    | 178 ++++++++++++++++++
 Documentation/sound/designs/index.rst         |   1 +
 .../kernel-api/writing-an-alsa-driver.rst     |  12 ++
 3 files changed, 191 insertions(+)
 create mode 100644 Documentation/sound/designs/card-names.rst

diff --git a/Documentation/sound/designs/card-names.rst b/Documentation/sound/designs/card-names.rst
new file mode 100644
index 0000000000..a4c0a638d1
--- /dev/null
+++ b/Documentation/sound/designs/card-names.rst
@@ -0,0 +1,178 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==========
+Card Names
+==========
+
+Background
+==========
+
+A sound card carries three strings that exist purely to be shown to a human:
+``shortname``, ``longname`` and ``mixername`` in ``struct snd_card``, handed to
+user space as ``name``, ``longname`` and ``mixername`` in
+``struct snd_ctl_card_info``.  All three are fixed-size byte arrays, 32 and 80
+bytes, and a name that does not fit is cut.
+
+Those names are UTF-8.  That is not a new rule: USB has worked this way for
+decades, because ``usb_string()`` converts the UTF-16LE string descriptors of a
+device into UTF-8, so ``dev->product`` reaches the card name fields as UTF-8
+already.
+
+Two things follow from the fixed size, and this document is about both.
+
+First, a cut can land in the middle of a multi-byte character, leaving a string
+that is not valid UTF-8 and that user space cannot repair, because the missing
+bytes never left the kernel.  ``snd_utf8_strscpy()`` fixes that: it is
+``strscpy()`` except that it ends the copy after the last complete sequence
+that fits.  Drivers that copy a device-provided name into one of these fields
+should use it.
+
+Second, cutting cleanly is still cutting.  32 bytes is eight CJK characters, so
+a name can be valid and still be unusable.  For that, ``struct snd_card`` also
+carries the untruncated form of each name:
+
+======================  ==========================  ===============================
+fixed field             untruncated form            user space type
+======================  ==========================  ===============================
+``shortname``           ``full_shortname``          ``SND_CTL_CARD_BTYPE_NAME``
+``longname``            ``full_longname``           ``SND_CTL_CARD_BTYPE_LONGNAME``
+``mixername``           ``full_mixername``          ``SND_CTL_CARD_BTYPE_MIXERNAME``
+======================  ==========================  ===============================
+
+This mirrors ``card->components``, which holds the whole components string of
+which ``snd_ctl_card_info::components`` is a cut copy, and which user space
+reads through the same ioctl.
+
+``id``, ``driver`` and ``components`` have no untruncated counterpart of this
+kind.  They are not display names: ``id`` is the unique handle of a card and
+part of path names, ``driver`` is the key alsa-lib matches configuration
+against, and ``components`` already has its own.  ``id`` in particular must
+stay alphanumeric ASCII, and the kernel enforces that when deriving it.
+
+Rules
+=====
+
+One name, two lengths
+    The untruncated field is not a second name.  It is the same name, not cut.
+    It stays NULL when the name fits, so "set" always means "the fixed field is
+    not the whole story", and there is never a pair that could disagree.
+
+Either may be empty
+    A driver may fill only the fixed field, which is what nearly all of them do
+    today, and a card may have no mixername at all.  No reader may assume a
+    name is present.
+
+Names are not identifiers
+    Homoglyph detection is impossible in the kernel and NFC is not enforced, so
+    two different cards can legitimately present names that look identical.
+    Anything that matches, authorises or picks a device must use ``id``,
+    ``driver`` or ``components``.
+
+NFC is a recommendation
+    Drivers should hand over names in Normalization Form C.  The kernel cannot
+    normalise: that needs the Unicode tables of ``CONFIG_UNICODE``, which the
+    sound core has no reason to depend on.  A reader that compares names should
+    normalise them itself.
+
+What the kernel guarantees, and what it does not
+================================================
+
+For every name it hands out, the kernel guarantees that the string:
+
+* is well-formed UTF-8, and is never cut inside a multi-byte sequence -
+  neither in the fixed field nor in the untruncated one;
+* carries no code point that would break a line, that is no C0 or C1 control
+  character and no U+2028 or U+2029.
+
+The second one is not cosmetic: the kernel prints these names into
+``/proc/asound/cards``, which is line based, so a newline in a name would
+corrupt a file that user space parses.  That is also why the repair belongs in
+the kernel rather than in alsa-lib.
+
+Nothing beyond that is guaranteed, and that is a deliberate choice rather than
+an oversight.  Invisible and bidirectional formatting characters, deprecated
+format characters, the tag block, variation selectors and non-characters all
+pass through untouched.  Catching them needs the Unicode character properties
+(``Default_Ignorable_Code_Point`` and the general categories), which would mean
+carrying a table in the sound core that goes stale with every Unicode release.
+A hand-written list of ranges is worse than none: it looks like a defence while
+leaving gaps, and user space then skips its own filtering.
+
+So the division of labour is:
+
+=====================================  ==============================
+the kernel                             user space
+=====================================  ==============================
+well-formed UTF-8                      NFC normalisation
+one line, no control characters        ``Default_Ignorable`` filtering
+never cut inside a sequence            bidi isolation for display
+=====================================  ==============================
+
+Concretely, before showing a name, wrap it in U+2066/U+2069 so that strong
+right-to-left characters cannot reorder the text around it, and drop
+``Default_Ignorable_Code_Point``.
+
+Rather than rejecting a name it dislikes, the kernel repairs it: a malformed
+sequence becomes U+FFFD, one per bad byte as recommended for a maximal subpart
+in Unicode TR#36, and a line breaking code point becomes a space.  Rejection
+would achieve nothing here: the fixed field still holds the name, and readers
+are told to use that whenever the untruncated one is absent, so a rejected name
+would reach them anyway.
+
+Driver side
+===========
+
+Copy a device-provided name into the fixed field with ``snd_utf8_strscpy()``,
+then offer the untruncated form::
+
+  snd_utf8_strscpy(card->shortname, product, sizeof(card->shortname));
+  snd_card_set_full_shortname(card, product);
+
+Order matters: the setter compares against ``card->shortname`` and stores
+nothing if the name fit, so the fixed field has to be there first.  A driver
+whose names are compile-time ASCII literals, which is most of them, needs
+neither call.
+
+The setters strip surrounding whitespace, cut an over-long name at a character
+boundary, and treat an empty result as "unset".  Passing NULL clears a field.
+They do not reject anything: the only error is ``-ENOMEM``.  The size limits
+are four times the size of the corresponding fixed field
+(``SNDRV_CARD_FULL_SHORTNAME_MAX`` and friends), so the same number of
+characters always fits, whatever they encode to.
+
+User space side
+===============
+
+The untruncated names are read through the existing
+``SNDRV_CTL_IOCTL_CARD_BYTES`` ioctl, which already carries the components
+string the same way.  No existing structure or ioctl changes, so an old binary
+keeps working unchanged.
+
+Call it once with ``data`` set to NULL to learn the length, then once with a
+buffer of that size::
+
+  struct snd_ctl_card_bytes b = { .type = SND_CTL_CARD_BTYPE_NAME };
+
+  if (ioctl(fd, SNDRV_CTL_IOCTL_CARD_BYTES, &b) < 0) {
+          /* kernel too old: the fixed field is all there is */
+          return use_fixed_name();
+  }
+
+  buf = malloc(b.data_len);
+  if (!buf)
+          return -ENOMEM;
+  b.data_allocated = b.data_len;
+  b.data = (__u64)(uintptr_t)buf;
+  if (ioctl(fd, SNDRV_CTL_IOCTL_CARD_BYTES, &b) < 0) {
+          free(buf);
+          return use_fixed_name();
+  }
+
+An empty result means the name fit, so ``snd_ctl_card_info`` already has it.
+An ``-EINVAL`` from a kernel that does not know the type means the same thing.
+``SNDRV_CTL_IOCTL_PVERSION`` reports at least 2.0.11 where the types exist.
+
+There is deliberately no sysfs interface for now.  It would be a second piece
+of ABI for the same data, and nothing in user space has asked for one yet; it
+can be added later if udev rules turn out to need the names before the control
+device is opened.
diff --git a/Documentation/sound/designs/index.rst b/Documentation/sound/designs/index.rst
index 6b825c5617..e9bc10c81f 100644
--- a/Documentation/sound/designs/index.rst
+++ b/Documentation/sound/designs/index.rst
@@ -5,6 +5,7 @@ Designs and Implementations
    :maxdepth: 2
 
    control-names
+   card-names
    channel-mapping-api
    compress-accel
    compress-offload
diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst
index 86b64de031..86796297c8 100644
--- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst
+++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst
@@ -438,6 +438,18 @@ functionality of each chip type.
 The shortname field is a string shown as more verbose name. The longname
 field contains the information shown in ``/proc/asound/cards``.
 
+These fields, together with mixername, are fixed-size and hold UTF-8. A name
+that comes from the device rather than from a literal in the driver needs two
+things: copy it with snd_utf8_strscpy() so that a name too long for the field
+is not cut in the middle of a character, and offer the untruncated form so
+that user space can get the whole name::
+
+  snd_utf8_strscpy(card->shortname, product, sizeof(card->shortname));
+  snd_card_set_full_shortname(card, product);
+
+The second call stores nothing if the name fit, so it costs nothing for short
+names. See Documentation/sound/designs/card-names.rst for the details.
+
 5) Create other components, such as mixer, MIDI, etc.
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.43.0