[PATCH] usb: gadget: f_uac2: fix invalid free in UAC2_RATE_ATTRIBUTE store

Deepanshu Kartikey <[email protected]>
Newsgroups org.kernel.vger.linux-usb,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
The store callback generated by UAC2_RATE_ATTRIBUTE() parses a
comma-separated list of sample rates by repeatedly calling strsep()
on split_page, which was obtained via kstrdup(). strsep() advances
the pointer it is given to point past each consumed token, so by
the time the loop reaches the end label - either after a normal
full parse or after an early exit via "goto end" when kstrtou32()
fails on a malformed token - split_page no longer points at the
start of the buffer returned by kstrdup(). kfree() is then called
on this interior pointer, which is not a valid allocation start
address, resulting in a KASAN invalid-free.

Fix this by keeping a separate pointer, orig_page, that is set
once from the kstrdup() return value and is never modified by
strsep(). orig_page is initialized to NULL so that early exits
before kstrdup() is reached (e.g. the opts->refcnt busy check)
result in a harmless kfree(NULL) instead of freeing an
uninitialized stack value. kfree() is called on orig_page instead
of split_page at the end label.

Since both f_uac2_opts_p_srate_store() and f_uac2_opts_c_srate_store()
are generated from this same macro, this fixes both attributes.

Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=a4f65284f1451010b0a8
Fixes: a7339e4f5788 ("usb: gadget: f_uac2: Support multiple sampling rates")
Tested-by: [email protected]
Signed-off-by: Deepanshu Kartikey <[email protected]>
---
 drivers/usb/gadget/function/f_uac2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 897787d0803c..4b403cb779fd 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -2012,7 +2012,7 @@ static ssize_t f_uac2_opts_##name##_store(struct config_item *item,	\
 					  const char *page, size_t len)	\
 {									\
 	struct f_uac2_opts *opts = to_f_uac2_opts(item);		\
-	char *split_page = NULL;					\
+	char *split_page = NULL, *orig_page = NULL;					\
 	int ret = -EINVAL;						\
 	char *token;							\
 	u32 num;							\
@@ -2026,7 +2026,7 @@ static ssize_t f_uac2_opts_##name##_store(struct config_item *item,	\
 									\
 	i = 0;								\
 	memset(opts->name##s, 0x00, sizeof(opts->name##s));		\
-	split_page = kstrdup(page, GFP_KERNEL);				\
+	orig_page = split_page = kstrdup(page, GFP_KERNEL);				\
 	while ((token = strsep(&split_page, ",")) != NULL) {		\
 		ret = kstrtou32(token, 0, &num);			\
 		if (ret)						\
@@ -2037,7 +2037,7 @@ static ssize_t f_uac2_opts_##name##_store(struct config_item *item,	\
 	};								\
 									\
 end:									\
-	kfree(split_page);						\
+	kfree(orig_page);						\
 	mutex_unlock(&opts->lock);					\
 	return ret;							\
 }									\
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.