[PATCH v2] usb: gadget: f_uac1_legacy: remove broken string configfs attributes

[PATCH v2] usb: gadget: f_uac1_legacy: remove broken string configfs attributes

[Xu Yang]

From: Xu Yang <xu.yang_2@nxp.com>

The UAC1_STR_ATTRIBUTE macro defines configfs show/store handlers for
the fn_play, fn_cap, and fn_cntl string options. The store function
contains an inverted null check on the kstrndup() return value.

This means every write attempt returns -ENOMEM on success and
dereferences a NULL pointer on allocation failure. The attributes
have been broken and unused for many years.

Remove the UAC1_STR_ATTRIBUTE macro and the three attributes it
generated. The internal defaults (FILE_PCM_PLAYBACK, FILE_PCM_CAPTURE,
FILE_CONTROL) set in f_audio_alloc_inst() are unaffected.

Fixes: 0854611a19ae ("usb: gadget: f_uac1: add configfs support")
Link: https://lore.kernel.org/linux-usb/20260625113154.1954813-1-xu.yang_2@oss.nxp.com/
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Assisted-by: Claude:claude-sonnet-4.6
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - Remove fn_play, fn_cap, and fn_cntl in gadget-testing.rst
 - Remove fn_*_alloc variable
---
 .../testing/configfs-usb-gadget-uac1_legacy   |  3 -
 Documentation/usb/gadget-testing.rst          |  3 -
 drivers/usb/gadget/function/f_uac1_legacy.c   | 56 -------------------
 drivers/usb/gadget/function/u_uac1_legacy.h   |  3 -
 4 files changed, 65 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy b/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy
index b2eaefd9bc49..6a681d219f43 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uac1_legacy
@@ -5,8 +5,5 @@ Description:
 		The attributes:
 
 		audio_buf_size - audio buffer size
-		fn_cap - capture pcm device file name
-		fn_cntl - control device file name
-		fn_play - playback pcm device file name
 		req_buf_size - ISO OUT endpoint request buffer size
 		req_count - ISO OUT endpoint request count
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index a6e8292f320a..4921e5307d49 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -714,9 +714,6 @@ The uac1 function provides these attributes in its function directory:
 
 	=============== ====================================
 	audio_buf_size	audio buffer size
-	fn_cap		capture pcm device file name
-	fn_cntl		control device file name
-	fn_play		playback pcm device file name
 	req_buf_size	ISO OUT endpoint request buffer size
 	req_count	ISO OUT endpoint request count
 	=============== ====================================
diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
index 5d201a2e30e7..3f52099a4fdd 100644
--- a/drivers/usb/gadget/function/f_uac1_legacy.c
+++ b/drivers/usb/gadget/function/f_uac1_legacy.c
@@ -888,60 +888,10 @@ UAC1_INT_ATTRIBUTE(req_buf_size);
 UAC1_INT_ATTRIBUTE(req_count);
 UAC1_INT_ATTRIBUTE(audio_buf_size);
 
-#define UAC1_STR_ATTRIBUTE(name)					\
-static ssize_t f_uac1_opts_##name##_show(struct config_item *item,	\
-					 char *page)			\
-{									\
-	struct f_uac1_legacy_opts *opts = to_f_uac1_opts(item);		\
-	int result;							\
-									\
-	mutex_lock(&opts->lock);					\
-	result = sprintf(page, "%s\n", opts->name);			\
-	mutex_unlock(&opts->lock);					\
-									\
-	return result;							\
-}									\
-									\
-static ssize_t f_uac1_opts_##name##_store(struct config_item *item,	\
-					  const char *page, size_t len)	\
-{									\
-	struct f_uac1_legacy_opts *opts = to_f_uac1_opts(item);		\
-	int ret = -EBUSY;						\
-	char *tmp;							\
-									\
-	mutex_lock(&opts->lock);					\
-	if (opts->refcnt)						\
-		goto end;						\
-									\
-	tmp = kstrndup(page, len, GFP_KERNEL);				\
-	if (tmp) {							\
-		ret = -ENOMEM;						\
-		goto end;						\
-	}								\
-	if (opts->name##_alloc)						\
-		kfree(opts->name);					\
-	opts->name##_alloc = true;					\
-	opts->name = tmp;						\
-	ret = len;							\
-									\
-end:									\
-	mutex_unlock(&opts->lock);					\
-	return ret;							\
-}									\
-									\
-CONFIGFS_ATTR(f_uac1_opts_, name)
-
-UAC1_STR_ATTRIBUTE(fn_play);
-UAC1_STR_ATTRIBUTE(fn_cap);
-UAC1_STR_ATTRIBUTE(fn_cntl);
-
 static struct configfs_attribute *f_uac1_attrs[] = {
 	&f_uac1_opts_attr_req_buf_size,
 	&f_uac1_opts_attr_req_count,
 	&f_uac1_opts_attr_audio_buf_size,
-	&f_uac1_opts_attr_fn_play,
-	&f_uac1_opts_attr_fn_cap,
-	&f_uac1_opts_attr_fn_cntl,
 	NULL,
 };
 
@@ -956,12 +906,6 @@ static void f_audio_free_inst(struct usb_function_instance *f)
 	struct f_uac1_legacy_opts *opts;
 
 	opts = container_of(f, struct f_uac1_legacy_opts, func_inst);
-	if (opts->fn_play_alloc)
-		kfree(opts->fn_play);
-	if (opts->fn_cap_alloc)
-		kfree(opts->fn_cap);
-	if (opts->fn_cntl_alloc)
-		kfree(opts->fn_cntl);
 	kfree(opts);
 }
 
diff --git a/drivers/usb/gadget/function/u_uac1_legacy.h b/drivers/usb/gadget/function/u_uac1_legacy.h
index b5df9bcbbeba..b9ddae550ff3 100644
--- a/drivers/usb/gadget/function/u_uac1_legacy.h
+++ b/drivers/usb/gadget/function/u_uac1_legacy.h
@@ -62,9 +62,6 @@ struct f_uac1_legacy_opts {
 	char				*fn_cap;
 	char				*fn_cntl;
 	unsigned			bound:1;
-	unsigned			fn_play_alloc:1;
-	unsigned			fn_cap_alloc:1;
-	unsigned			fn_cntl_alloc:1;
 	struct mutex			lock;
 	int				refcnt;
 };
-- 
2.34.1

Re: [PATCH v2] usb: gadget: f_uac1_legacy: remove broken string configfs attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds checking on integer configfs attributes leads to ZERO_SIZE_PTR dereference and kernel crash.
- [High] Permanent memory and request leak when the USB host changes alternate settings.
--

--- Patch [1]: [PATCH v2] usb: gadget: f_uac1_legacy: remove broken string configfs attributes ---
commit 4b8c64c0b84575a689f911f2adeef96bb30cecea
Author: Xu Yang <xu.yang_2@nxp.com>

usb: gadget: f_uac1_legacy: remove broken string configfs attributes

The UAC1_STR_ATTRIBUTE macro defines configfs show/store handlers for
the fn_play, fn_cap, and fn_cntl string options. The store function
contains an inverted null check on the kstrndup() return value. This
patch removes the UAC1_STR_ATTRIBUTE macro and the three attributes it
generated, as they have been broken and unused for many years.

Link: https://lore.kernel.org/linux-usb/20260625113154.1954813-1-xu.yang_2@oss.nxp.com/

> diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
> index 5d201a2e30e7f..3f52099a4fdd5 100644
> --- a/drivers/usb/gadget/function/f_uac1_legacy.c
> +++ b/drivers/usb/gadget/function/f_uac1_legacy.c
> @@ -888,60 +888,10 @@ UAC1_INT_ATTRIBUTE(req_buf_size);
>  UAC1_INT_ATTRIBUTE(req_count);
>  UAC1_INT_ATTRIBUTE(audio_buf_size);

[Severity: High]
This isn't a bug introduced by this patch, but does this lack of bounds
checking allow audio_buf_size to be set to 0?

If a user with configfs access sets audio_buf_size to 0, it appears
f_audio_buffer_alloc() will call kzalloc(0) which returns ZERO_SIZE_PTR:

drivers/usb/gadget/function/f_uac1_legacy.c:f_audio_buffer_alloc() {
	...
	copy_buf->buf = kzalloc(buf_size, GFP_ATOMIC);
	if (!copy_buf->buf) {
	...
}

This bypasses the NULL check. Later, when an audio out packet is received,
f_audio_out_ep_complete() attempts to memcpy the incoming data:

drivers/usb/gadget/function/f_uac1_legacy.c:f_audio_out_ep_complete() {
	...
	memcpy(copy_buf->buf + copy_buf->actual, req->buf, req->actual);
	...
}

Can this copy into the ZERO_SIZE_PTR address cause an immediate kernel panic?

> -#define UAC1_STR_ATTRIBUTE(name)					\
> -static ssize_t f_uac1_opts_##name##_show(struct config_item *item,	\

[ ... ]

> @@ -956,12 +906,6 @@ static void f_audio_free_inst(struct usb_function_instance *f)
>  	struct f_uac1_legacy_opts *opts;
>  
>  	opts = container_of(f, struct f_uac1_legacy_opts, func_inst);
> -	if (opts->fn_play_alloc)
> -		kfree(opts->fn_play);
> -	if (opts->fn_cap_alloc)
> -		kfree(opts->fn_cap);
> -	if (opts->fn_cntl_alloc)
> -		kfree(opts->fn_cntl);
>  	kfree(opts);
>  }

[Severity: High]
This is a pre-existing issue, but does f_audio_disable() leak memory and USB
requests when the host changes alternate settings?

When the USB host selects alternate setting 1, f_audio_set_alt() allocates
requests and buffers:

drivers/usb/gadget/function/f_uac1_legacy.c:f_audio_set_alt() {
	...
	req = usb_ep_alloc_request(out_ep, GFP_ATOMIC);
	if (req) {
		req->buf = kzalloc(req_buf_size, GFP_ATOMIC);
	...
}

However, the disable callback appears to be empty and fails to free these
resources:

drivers/usb/gadget/function/f_uac1_legacy.c:f_audio_disable() {
	return;
}

Could an untrusted USB host trigger this repeatedly by changing the alternate
setting back and forth, exhausting device memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713060845.3759673-1-xu.yang_2@oss.nxp.com?part=1