From: Jared Kangas <kangas.jd@gmail.com>
To: vaibhav.sr@gmail.com
Cc: mgreer@animalcreek.com, elder@kernel.org,
gregkh@linuxfoundation.org, greybus-dev@lists.linaro.org,
johan@kernel.org, linux-kernel@vger.kernel.org,
linux-staging@lists.linux.dev, kangas.jd@gmail.com
Subject: [PATCH] staging: greybus: audio: fix loop cursor use after iteration
Date: Sun, 5 Jun 2022 16:18:06 -0700 [thread overview]
Message-ID: <20220605231806.720085-1-kangas.jd@gmail.com> (raw)
gbaudio_dapm_free_controls() iterates over widgets using
list_for_each_entry_safe(), which leaves the loop cursor pointing to a
meaningless structure if it completes a traversal of the list. The
cursor was set to NULL at the end of the loop body, but would be
overwritten by the final loop cursor update.
Because of this behavior, the widget could be non-null after the loop
even if the widget wasn't found, and the cleanup logic would treat the
pointer as a valid widget to free.
To fix this, introduce a temporary variable to act as the loop cursor
and copy it to a variable that can be accessed after the loop finishes.
This was detected with the help of Coccinelle.
Signed-off-by: Jared Kangas <kangas.jd@gmail.com>
---
drivers/staging/greybus/audio_helper.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/greybus/audio_helper.c b/drivers/staging/greybus/audio_helper.c
index 843760675876..07461a5d97c7 100644
--- a/drivers/staging/greybus/audio_helper.c
+++ b/drivers/staging/greybus/audio_helper.c
@@ -115,7 +115,7 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
int num)
{
int i;
- struct snd_soc_dapm_widget *w, *next_w;
+ struct snd_soc_dapm_widget *w, *next_w, *tmp_w;
#ifdef CONFIG_DEBUG_FS
struct dentry *parent = dapm->debugfs_dapm;
struct dentry *debugfs_w = NULL;
@@ -124,13 +124,14 @@ int gbaudio_dapm_free_controls(struct snd_soc_dapm_context *dapm,
mutex_lock(&dapm->card->dapm_mutex);
for (i = 0; i < num; i++) {
/* below logic can be optimized to identify widget pointer */
- list_for_each_entry_safe(w, next_w, &dapm->card->widgets,
+ w = NULL;
+ list_for_each_entry_safe(tmp_w, next_w, &dapm->card->widgets,
list) {
- if (w->dapm != dapm)
- continue;
- if (!strcmp(w->name, widget->name))
+ if (tmp_w->dapm == dapm &&
+ !strcmp(tmp_w->name, widget->name)) {
+ w = tmp_w;
break;
- w = NULL;
+ }
}
if (!w) {
dev_err(dapm->dev, "%s: widget not found\n",
--
2.34.3
next reply other threads:[~2022-06-05 23:18 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-05 23:18 Jared Kangas [this message]
2022-06-06 13:06 ` [PATCH] staging: greybus: audio: fix loop cursor use after iteration Dan Carpenter
2022-06-07 16:40 ` Jared Kangas
2022-06-07 18:39 ` Dan Carpenter
2022-06-07 19:06 ` Alex Elder
2022-06-07 16:36 ` Johan Hovold
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220605231806.720085-1-kangas.jd@gmail.com \
--to=kangas.jd@gmail.com \
--cc=elder@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=johan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=mgreer@animalcreek.com \
--cc=vaibhav.sr@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox