[PATCH] wifi: mac80211: fix unassigned variable access

[PATCH] wifi: mac80211: fix unassigned variable access

[Antonio Quartulli]

In ieee80211_latest_active_link_conn_timeout() we loop over all
sta->links in order to compute the timeout expiring last across
all links.

Such timeout is stored in `latest_timeout` which is used in the
time_after() comparison before having been initialized.

Should the for-loop terminate without ever setting `latest_timeout`
we would even return it in its uninitialized state.

Fix this behaviour by initializing the variable to its minimum
value 0.

Address-Coverity-ID: 1647986 ("Uninitialized variables (UNINIT)")
Fixes: 1bc892d76a6f ("wifi: mac80211: extend connection monitoring for MLO")
Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
---
 net/mac80211/mlme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index b4b7ea52c65e..0d96490510bf 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -8521,7 +8521,7 @@ static void ieee80211_sta_bcn_mon_timer(struct timer_list *t)
 static unsigned long
 ieee80211_latest_active_link_conn_timeout(struct ieee80211_sub_if_data *sdata)
 {
-	unsigned long latest_timeout;
+	unsigned long latest_timeout = 0;
 	unsigned int link_id;
 	struct sta_info *sta;
 
-- 
2.49.1

Re: [PATCH] wifi: mac80211: fix unassigned variable access

[Johannes Berg]

On Mon, 2025-07-21 at 23:17 +0200, Antonio Quartulli wrote:
> In ieee80211_latest_active_link_conn_timeout() we loop over all
> sta->links in order to compute the timeout expiring last across
> all links.
> 
> Such timeout is stored in `latest_timeout` which is used in the
> time_after() comparison before having been initialized.
> 
> Should the for-loop terminate without ever setting `latest_timeout`
> we would even return it in its uninitialized state.

It's really not plausible to have no links at all though.

So I think if anything we should worry about the 

                if (time_is_after_jiffies(timeout) &&
                    time_after(timeout, latest_timeout))
                        latest_timeout = timeout;

comparison, but for that just unconditionally setting it to 0 is really
the wrong thing to do, since it means you compare to an arbitrary time
zero here?

johannes

Re: [PATCH] wifi: mac80211: fix unassigned variable access

[Antonio Quartulli]

On 22/07/2025 10:54, Johannes Berg wrote:
> On Mon, 2025-07-21 at 23:17 +0200, Antonio Quartulli wrote:
>> In ieee80211_latest_active_link_conn_timeout() we loop over all
>> sta->links in order to compute the timeout expiring last across
>> all links.
>>
>> Such timeout is stored in `latest_timeout` which is used in the
>> time_after() comparison before having been initialized.
>>
>> Should the for-loop terminate without ever setting `latest_timeout`
>> we would even return it in its uninitialized state.
> 
> It's really not plausible to have no links at all though.

I imagined so.

> 
> So I think if anything we should worry about the
> 
>                  if (time_is_after_jiffies(timeout) &&
>                      time_after(timeout, latest_timeout))
>                          latest_timeout = timeout;
> 
> comparison, but for that just unconditionally setting it to 0 is really
> the wrong thing to do, since it means you compare to an arbitrary time
> zero here?

We are looking for the maximum timeout, so using 0 (minimum possible 
value) as base line should be what we want, no?

[note: I am assuming 0 is not a possible timeout value]

Alternatively, we can just skip evaluating time_after() when 
latest_timeout is 0.

Regards,

-- 
Antonio Quartulli

CEO and Co-Founder
Mandelbit Srl
https://www.mandelbit.com

Re: [PATCH] wifi: mac80211: fix unassigned variable access

[Johannes Berg]

On Tue, 2025-07-22 at 11:00 +0200, Antonio Quartulli wrote:
> > So I think if anything we should worry about the
> > 
> >                  if (time_is_after_jiffies(timeout) &&
> >                      time_after(timeout, latest_timeout))
> >                          latest_timeout = timeout;
> > 
> > comparison, but for that just unconditionally setting it to 0 is really
> > the wrong thing to do, since it means you compare to an arbitrary time
> > zero here?
> 
> We are looking for the maximum timeout, so using 0 (minimum possible 
> value) as base line should be what we want, no?
> 
> [note: I am assuming 0 is not a possible timeout value]

That assumption is wrong, in fact jiffies starts at a slightly negative
value (about 5 minutes?) so that a few minutes after boot the time goes
to the perfectly valid value zero. And on HZ=1000 32-bit systems, this
situation of course reappears every ~49.7 days.
> 
> Alternatively, we can just skip evaluating time_after() when 
> latest_timeout is 0.

But then we'd also have to set it to 1 if a timeout value is actually 0.

johannes

Re: [PATCH] wifi: mac80211: fix unassigned variable access

[Antonio Quartulli]

On 22/07/2025 11:02, Johannes Berg wrote:
> On Tue, 2025-07-22 at 11:00 +0200, Antonio Quartulli wrote:
>>> So I think if anything we should worry about the
>>>
>>>                   if (time_is_after_jiffies(timeout) &&
>>>                       time_after(timeout, latest_timeout))
>>>                           latest_timeout = timeout;
>>>
>>> comparison, but for that just unconditionally setting it to 0 is really
>>> the wrong thing to do, since it means you compare to an arbitrary time
>>> zero here?
>>
>> We are looking for the maximum timeout, so using 0 (minimum possible
>> value) as base line should be what we want, no?
>>
>> [note: I am assuming 0 is not a possible timeout value]
> 
> That assumption is wrong, in fact jiffies starts at a slightly negative
> value (about 5 minutes?) so that a few minutes after boot the time goes
> to the perfectly valid value zero. And on HZ=1000 32-bit systems, this
> situation of course reappears every ~49.7 days.

As discussed on IRC, also the caller of this function is wrongly 
assuming that returning 0 means "no timeout planned".

For this reason I'll send v2 where I will initialize latest_timeout to 
jiffies and I will re-arrange surrounding checks accordingly (caller 
included).


Regards,

-- 
Antonio Quartulli

CEO and Co-Founder
Mandelbit Srl
https://www.mandelbit.com