Hi,
On 23/04/2025 20:29, Dan Carpenter wrote:
On Wed, Apr 23, 2025 at 05:25:44PM +0200, Romain Gantois wrote:
Hello Dan,
On Wednesday, 23 April 2025 10:21:18 CEST Dan Carpenter wrote:
When the list_for_each_entry_reverse() exits without hitting a break
then the list cursor points to invalid memory. So this check for
if (c2a->fixed) is checking bogus memory. Fix it by using a "found"
variable to track if we found what we were looking for or not.
IIUC the for loop ending condition in list_for_each_entry_reverse() is
"!list_entry_is_head(pos, head, member);", so even if the loop runs to
completion, the pointer should still be valid right?
head is &chan->alias_pairs. pos is an offset off the head. In this
case, the offset is zero. So it's &chan->alias_pairs minus zero.
So we exit the list with c2a = (void *)&chan->alias_pairs.
If you look how struct i2c_atr_chan is declareted the next struct member
after alias_pairs is:
struct i2c_atr_alias_pool *alias_pool;
So if (c2a->fixed) is poking around in the alias_pool pointer. It's not
out of bounds but it's not valid either.
Maybe it's just me, but I had hard time following that explanation. So
here's mine:
The list head (i2c_atr_chan.alias_pairs) is not a full entry, it's just
a struct list_head. When the for loop runs to completion, c2a doesn't
point to a struct i2c_atr_alias_pair, so you can't access c2a->fixed.
For the patch:
Reviewed-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
Tomi
