When list_for_each_entry() completes the iteration over the whole list without breaking the loop, the iterator value will be a bogus pointer computed based on the head element.

While it is safe to use the pointer to determine if it was computed based on the head element, either with list_entry_is_head() or &pos->member == head, using the iterator variable after the loop should be avoided.

In preparation to limiting the scope of a list iterator to the list traversal loop, use a dedicated pointer to point to the found element [1].

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWX... [1] Signed-off-by: Jakob Koschel jakobkoschel@gmail.com --- sound/soc/sprd/sprd-mcdt.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/sound/soc/sprd/sprd-mcdt.c b/sound/soc/sprd/sprd-mcdt.c index f6a55fa60c1b..6e27789a0df5 100644 --- a/sound/soc/sprd/sprd-mcdt.c +++ b/sound/soc/sprd/sprd-mcdt.c @@ -866,20 +866,19 @@ EXPORT_SYMBOL_GPL(sprd_mcdt_chan_dma_disable); struct sprd_mcdt_chan *sprd_mcdt_request_chan(u8 channel, enum sprd_mcdt_channel_type type) { - struct sprd_mcdt_chan *temp; + struct sprd_mcdt_chan *temp = NULL; + struct sprd_mcdt_chan *iter;

mutex_lock(&sprd_mcdt_list_mutex);

- list_for_each_entry(temp, &sprd_mcdt_chan_list, list) { - if (temp->type == type && temp->id == channel) { - list_del_init(&temp->list); + list_for_each_entry(iter, &sprd_mcdt_chan_list, list) { + if (iter->type == type && iter->id == channel) { + list_del_init(&iter->list); + temp = iter; break; } }

- if (list_entry_is_head(temp, &sprd_mcdt_chan_list, list)) - temp = NULL; - mutex_unlock(&sprd_mcdt_list_mutex);

return temp;

To move the list iterator variable into the list_for_each_entry_*() macro in the future it should be avoided to use the list iterator variable after the loop body.

To *never* use the list iterator variable after the loop it was concluded to use a separate iterator variable instead of a found boolean [1].

This removes the need to use a found variable and simply checking if the variable was set, can determine if the break/goto was hit.

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWX... [1] Signed-off-by: Jakob Koschel jakobkoschel@gmail.com --- drivers/misc/atmel-ssc.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c index d6cd5537126c..343e25555fbb 100644 --- a/drivers/misc/atmel-ssc.c +++ b/drivers/misc/atmel-ssc.c @@ -25,25 +25,24 @@ static LIST_HEAD(ssc_list);

struct ssc_device *ssc_request(unsigned int ssc_num) { - int ssc_valid = 0; - struct ssc_device *ssc; + struct ssc_device *ssc = NULL, *iter;

mutex_lock(&user_lock); - list_for_each_entry(ssc, &ssc_list, list) { - if (ssc->pdev->dev.of_node) { - if (of_alias_get_id(ssc->pdev->dev.of_node, "ssc") + list_for_each_entry(iter, &ssc_list, list) { + if (iter->pdev->dev.of_node) { + if (of_alias_get_id(iter->pdev->dev.of_node, "ssc") == ssc_num) { - ssc->pdev->id = ssc_num; - ssc_valid = 1; + iter->pdev->id = ssc_num; + ssc = iter; break; } - } else if (ssc->pdev->id == ssc_num) { - ssc_valid = 1; + } else if (iter->pdev->id == ssc_num) { + ssc = iter; break; } }

- if (!ssc_valid) { + if (!ssc) { mutex_unlock(&user_lock); pr_err("ssc: ssc%d platform device is missing\n", ssc_num); return ERR_PTR(-ENODEV);

On 2022-03-31 11:50 PM, Jakob Koschel wrote:

When list_for_each_entry() completes the iteration over the whole list without breaking the loop, the iterator value will be a bogus pointer computed based on the head element.

While it is safe to use the pointer to determine if it was computed based on the head element, either with list_entry_is_head() or &pos->member == head, using the iterator variable after the loop should be avoided.

In preparation to limiting the scope of a list iterator to the list traversal loop, use a dedicated pointer to point to the found element [1].

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWX... [1] Signed-off-by: Jakob Koschel jakobkoschel@gmail.com

Thank you for taking time and updating catpt-driver along the way.

Acked-by: Cezary Rojewski cezary.rojewski@intel.com

---

sound/soc/intel/catpt/pcm.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/sound/soc/intel/catpt/pcm.c b/sound/soc/intel/catpt/pcm.c index a26000cd5ceb..b103b2d4026e 100644 --- a/sound/soc/intel/catpt/pcm.c +++ b/sound/soc/intel/catpt/pcm.c @@ -330,7 +330,8 @@ static int catpt_dai_apply_usettings(struct snd_soc_dai *dai, struct catpt_stream_runtime *stream) { struct snd_soc_component *component = dai->component;

• struct snd_kcontrol *pos;

• struct snd_kcontrol *pos = NULL;
• struct snd_kcontrol *iter; struct catpt_dev *cdev = dev_get_drvdata(dai->dev); const char *name; int ret;

@@ -354,12 +355,14 @@ static int catpt_dai_apply_usettings(struct snd_soc_dai *dai, return 0; }

• list_for_each_entry(pos, &component->card->snd_card->controls, list) {

• if (pos->private_data == component &&
  

•     !strncmp(name, pos->id.name, sizeof(pos->id.name)))
  

• list_for_each_entry(iter, &component->card->snd_card->controls, list) {

• if (iter->private_data == component &&
  

•     !strncmp(name, iter->id.name, sizeof(iter->id.name))) {
  

• 	pos = iter;
  break;
  

• }
  

  }

• if (list_entry_is_head(pos, &component->card->snd_card->controls, list))

• if (!pos) return -ENOENT;

  if (stream->template->type != CATPT_STRM_TYPE_LOOPBACK)

base-commit: f82da161ea75dc4db21b2499e4b1facd36dab275