Message ID | 20230109200910.146431-1-chaitanya.mgit@gmail.com |
---|---|
State | Accepted |
Headers | show |
Series | Fix BSS age underflow | expand |
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
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?
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
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 --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) {