linux-kernel - Re: [PATCH v3 3/4] ALSA: usb-audio: add module param device_quirk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ms6tchvf.wl-tiwai@suse.de>
Date: Wed, 17 Sep 2025 15:45:40 +0200
From: Takashi Iwai <tiwai@...e.de>
To: cryolitia@...ontech.com,
	Cryolitia PukNgae via B4 Relay <devnull+cryolitia.uniontech.com@...nel.org>
Cc: Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>,
	Jonathan Corbet <corbet@....net>,
	Luis Chamberlain <mcgrof@...nel.org>,
	Petr Pavlu <petr.pavlu@...e.com>,
	Daniel Gomez <da.gomez@...nel.org>,
	Sami Tolvanen <samitolvanen@...gle.com>,
	linux-sound@...r.kernel.org,
	linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org,
	Mingcong Bai <jeffbai@...c.io>,
	Kexy Biscuit <kexybiscuit@...c.io>,
	Nie Cheng <niecheng1@...ontech.com>,
	Zhan Jun <zhanjun@...ontech.com>,
	Feng Yuan <fengyuan@...ontech.com>,
	qaqland <anguoli@...ontech.com>,
	kernel@...ontech.com,
	linux-modules@...r.kernel.org,
	Takashi Iwai <tiwai@...e.de>
Subject: Re: [PATCH v3 3/4] ALSA: usb-audio: add module param device_quirk_flags

On Wed, 17 Sep 2025 14:46:42 +0200,
Cryolitia PukNgae via B4 Relay wrote:
> 
> From: Cryolitia PukNgae <cryolitia@...ontech.com>
> 
> For apply and unapply quirk flags more flexibly though param and sysfs
> 
> Co-developed-by: Takashi Iwai <tiwai@...e.de>
> Signed-off-by: Cryolitia PukNgae <cryolitia@...ontech.com>

I think an easier approach would be to rather parse the string value
at each time when probing a device and seeking for options.

That is, let's code without a mutex at first (for the permission
0444):

void snd_usb_init_dynamic_quirks(int idx, struct snd_usb_audio *chip)
{
	....
	/* old style option found: the position-based integer value */
	if (quirk_flags[idx] &&
	    !kstrtou32(quirk_flags[idx], 0, &chip->quirk_flags)) {
		usb_audio_dbg(....);
		return;
	}

	/* take the default quirk from the quirk table */
	snd_usb_init_quirk_flags(chip);

	for (i = 0; i < ARRAY_SIZE(quirk_flags); i++) {
		if (quirk_flags[i] && *quirk_flags[i]) {
			err = parse_quirk_option(chip, quirk_flags[i]);
			if (err < 0)
				return;
		}
	}
}

and the parser would be something like:

static int parse_quirk_option(struct snd_usb_audio *chip, const char *str)
{
	char *val __free(kfree) = NULL;
	char *field;
	int pid, vid;

	if (!strchr(str, ':'))
		return 0;

	val = kstrdup(str, GFP_KERNEL);
	if (!val)
		return -ENOMEM;

	/* Each entry consists of VID:PID:flags */
	field = strsep(&p, ":");
	if (!field)
		return 0;
	if (strcmp(field, "*") == 0)
		vid = 0;
	else if (kstrtou16(field, 16, &vid))
		return 0; // can spew warning message, too

	field = strsep(&p, ":");
	if (!field)
		return 0;
	if (strcmp(field, "*") == 0)
		pid = 0;
	else if (kstrtou16(field, 16, &pid))
		return 0; // can spew warning message, too

	.... // evaluate the tokens, set or clear chip->quirk_flags accordingly
	return 0;
}

So you can just use the normal charp type for the parameters.

Once after this working, we may want to allow the dynamic module
parameter change.  Then make the parameter a special type just to take
the device_quirk_mutex at set callback, while the set callback simply
calls parm_set_charp() for the rest.  For get and free callbacks, we
can use param_get_charp() and param_free_charp() as is.
Finally, snd_usb_init_dynamic_quirks() takes the device_quirk_mutex,
and that's all.  No need for heavy cleanups or linked list handling.

Of course, it's a bit inefficient from the performance POV, but the
device probing is rather a very rare event, so the speed doesn't
matter at all.


thanks,

Takashi