[PATCH] net: dsa: sja1105: avoid use of type-confused pointer in sja1105_insert_gate_entry()

[Vladimir Oltean]

It appears that list_for_each_entry() leaks a type-confused pointer when
the iteration loop ends with no early break, since "*p" will no longer
point to a "struct sja1105_gate_entry", but rather to some memory in
front of "gating_cfg->entries".

This isn't actually a problem here, because if the element we insert has
the highest interval, therefore we never exit the loop early, "p->list"
(which is all that we use outside the loop) will in fact point to
"gating_cfg->entries" even though "p" itself is invalid.

Nonetheless, there are preparations to increase the safety of
list_for_each_entry() by making it impossible to use the encapsulating
structure of the iterator element outside the loop. So something needs
to change here before those preparations go in, even though this
constitutes legitimate use.

Make it clear that we are not dereferencing members of the encapsulating
"struct sja1105_gate_entry" outside the loop, by using the regular
list_for_each() iterator, and dereferencing the struct sja1105_gate_entry
only within the loop.

With list_for_each(), the iterator element at the end of the loop does
have a sane value in all cases, and we can just use that as the "head"
argument of list_add().

Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
---
 drivers/net/dsa/sja1105/sja1105_vl.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/sja1105/sja1105_vl.c b/drivers/net/dsa/sja1105/sja1105_vl.c
index c0e45b393fde..fe93c80fe5ef 100644
--- a/drivers/net/dsa/sja1105/sja1105_vl.c
+++ b/drivers/net/dsa/sja1105/sja1105_vl.c
@@ -27,9 +27,15 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
 	if (list_empty(&gating_cfg->entries)) {
 		list_add(&e->list, &gating_cfg->entries);
 	} else {
-		struct sja1105_gate_entry *p;
+		struct list_head *pos;
+
+		/* We cannot safely use list_for_each_entry()
+		 * because we dereference "pos" after the loop
+		 */
+		list_for_each(pos, &gating_cfg->entries) {
+			struct sja1105_gate_entry *p;
 
-		list_for_each_entry(p, &gating_cfg->entries, list) {
+			p = list_entry(pos, struct sja1105_gate_entry, list);
 			if (p->interval == e->interval) {
 				NL_SET_ERR_MSG_MOD(extack,
 						   "Gate conflict");
@@ -40,7 +46,7 @@ static int sja1105_insert_gate_entry(struct sja1105_gating_config *gating_cfg,
 			if (e->interval < p->interval)
 				break;
 		}
-		list_add(&e->list, p->list.prev);
+		list_add(&e->list, pos->prev);
 	}
 
 	gating_cfg->num_entries++;
-----------------------------[ cut here ]-----------------------------