diff mbox series

Fix BSS age underflow

Message ID 20230109200910.146431-1-chaitanya.mgit@gmail.com
State Accepted
Headers show
Series Fix BSS age underflow | expand

Commit Message

Krishna Chaitanya Jan. 9, 2023, 8:09 p.m. UTC 
From: Krishna T <krishna.t@nordicsemi.no>

While checking for stale BSSes, the current time is used as a basis and
then based on age the stale check time is calculated, but if this is
done too early in the boot and if either BOOTTIME/MONOTONIC (the one
Zephyr uses by default) are used then the stale check time underflows
and goes to future causing active BSS entries in the scan to be treated
as stale and flushed.

Fix this by adding a check before calculating stale time and ignore this
check till the system reaches the BSS expiration time (this would never
happen with REALTIME clock).

Signed-off-by: Krishna T <krishna.t@nordicsemi.no>
Signed-off-by: Sridhar Nuvusetty <sridhar.nuvusetty@nordicsemi.no>
---
 wpa_supplicant/bss.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Johannes Berg Jan. 10, 2023, 4:58 p.m. UTC | #1 
On Tue, 2023-01-10 at 01:39 +0530, chaitanya.mgit@gmail.com wrote:
> From: Krishna T <krishna.t@nordicsemi.no>
> 
> While checking for stale BSSes, the current time is used as a basis and
> then based on age the stale check time is calculated, but if this is
> done too early in the boot and if either BOOTTIME/MONOTONIC (the one
> Zephyr uses by default) are used then the stale check time underflows
> and goes to future causing active BSS entries in the scan to be treated
> as stale and flushed.
> 
> Fix this by adding a check before calculating stale time and ignore this
> check till the system reaches the BSS expiration time (this would never
> happen with REALTIME clock).
> 

Makes sense, the REALTIME mention kind of threw me off at first though.

But I think there are probably more cases? Maybe we need some kind of
helper for the actual check, something like "check_age(item_ts, now_ts,
age)"?

johannes
Krishna Chaitanya Jan. 10, 2023, 5:13 p.m. UTC | #2 
On Tue, Jan 10, 2023 at 10:28 PM Johannes Berg
<johannes@sipsolutions.net> wrote:
>
> On Tue, 2023-01-10 at 01:39 +0530, chaitanya.mgit@gmail.com wrote:
> > From: Krishna T <krishna.t@nordicsemi.no>
> >
> > While checking for stale BSSes, the current time is used as a basis and
> > then based on age the stale check time is calculated, but if this is
> > done too early in the boot and if either BOOTTIME/MONOTONIC (the one
> > Zephyr uses by default) are used then the stale check time underflows
> > and goes to future causing active BSS entries in the scan to be treated
> > as stale and flushed.
> >
> > Fix this by adding a check before calculating stale time and ignore this
> > check till the system reaches the BSS expiration time (this would never
> > happen with REALTIME clock).
> >
>
> Makes sense, the REALTIME mention kind of threw me off at first though.
:), I was trying to convey that in the case of REALTIME, as we use absolute time
-180seconds is still in the past, assuming there are no jumps.

> But I think there are probably more cases? Maybe we need some kind of
> helper for the actual check, something like "check_age(item_ts, now_ts,
> age)"?
Sure, but this patch should work with most clock_id's, only case I can think
of is, time jumps in the case of REALTIME, but I guess that affects most callers
that use os_reltime_before?

In which case I would suggest using MONOTONIC os_get_rel_time or adding a
new APIs os_get_montime or passing clock_id to os_get_rel_time?
Johannes Berg Jan. 10, 2023, 8:29 p.m. UTC | #3 
On Tue, 2023-01-10 at 22:43 +0530, Krishna Chaitanya wrote:
> 
> > But I think there are probably more cases? Maybe we need some kind of
> > helper for the actual check, something like "check_age(item_ts, now_ts,
> > age)"?
> Sure, but this patch should work with most clock_id's, only case I can think
> of is, time jumps in the case of REALTIME, but I guess that affects most callers
> that use os_reltime_before?
> 
> In which case I would suggest using MONOTONIC os_get_rel_time or adding a
> new APIs os_get_montime or passing clock_id to os_get_rel_time?

Oh no, I just meant there might be other cases of doing subtractions? A
quick grep suggested there might be, but now looking at it it seems they
might not be, so not sure.

johannes
Jouni Malinen Feb. 1, 2023, 4:56 p.m. UTC | #4 
On Tue, Jan 10, 2023 at 01:39:10AM +0530, chaitanya.mgit@gmail.com wrote:
> While checking for stale BSSes, the current time is used as a basis and
> then based on age the stale check time is calculated, but if this is
> done too early in the boot and if either BOOTTIME/MONOTONIC (the one
> Zephyr uses by default) are used then the stale check time underflows
> and goes to future causing active BSS entries in the scan to be treated
> as stale and flushed.
> 
> Fix this by adding a check before calculating stale time and ignore this
> check till the system reaches the BSS expiration time (this would never
> happen with REALTIME clock).

Thanks, applied.
diff mbox series

Patch

diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c
index e13783ce1..65b58bd79 100644
--- a/wpa_supplicant/bss.c
+++ b/wpa_supplicant/bss.c
@@ -936,6 +936,10 @@  void wpa_bss_flush_by_age(struct wpa_supplicant *wpa_s, int age)
 		return;
 
 	os_get_reltime(&t);
+
+	if (t.sec < age)
+		return; /* avoid underflow */
+
 	t.sec -= age;
 
 	dl_list_for_each_safe(bss, n, &wpa_s->bss, struct wpa_bss, list) {