Skip to content

Commit 4882dd6

Browse files
pabigotjukkar
authored andcommitted
net: if: fix error in calculating router expiration
The existing implementation is inconsistent in that checking for expired routers when a timeout is processed detects end-of-life correctly (when the remaining duration exceeds the signed maximum), but the calculation of time remaining before expiration uses only unsigned calculation. So when the set of routers is changed the newly calculated timeout will not recognize routers that have expired, and so those routers expired late. In the worst case if the only remaining router had expired the timer may be set for almost two months in the future. Refactor to calculate remaining time in one place and as a signed value. Change a function name to more clearly reflect what it does. Avoid unnecessary race conditions in k_work API. Signed-off-by: Peter Bigot <[email protected]>
1 parent acd43cb commit 4882dd6

File tree

1 file changed

+29
-18
lines changed

1 file changed

+29
-18
lines changed

subsys/net/ip/net_if.c

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -674,26 +674,38 @@ static void iface_router_notify_deletion(struct net_if_router *router,
674674
}
675675
}
676676

677+
static inline int32_t iface_router_ends(const struct net_if_router *router,
678+
uint32_t now)
679+
{
680+
uint32_t ends = router->life_start;
681+
682+
ends += MSEC_PER_SEC * router->lifetime;
683+
684+
/* Signed number of ms until router lifetime ends */
685+
return (int32_t)(ends - now);
686+
}
677687

678-
static void iface_router_run_timer(uint32_t current_time)
688+
static void iface_router_update_timer(uint32_t now)
679689
{
680690
struct net_if_router *router, *next;
681-
uint32_t new_timer = UINT_MAX;
682-
683-
if (k_delayed_work_remaining_get(&router_timer)) {
684-
k_delayed_work_cancel(&router_timer);
685-
}
691+
uint32_t new_delay = UINT32_MAX;
686692

687693
SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&active_router_timers,
688694
router, next, node) {
689-
uint32_t current_timer = router->life_start +
690-
(MSEC_PER_SEC * router->lifetime) - current_time;
695+
int32_t ends = iface_router_ends(router, now);
696+
697+
if (ends <= 0) {
698+
new_delay = 0;
699+
break;
700+
}
691701

692-
new_timer = MIN(current_timer, new_timer);
702+
new_delay = MIN((uint32_t)ends, new_delay);
693703
}
694704

695-
if (new_timer != UINT_MAX) {
696-
k_delayed_work_submit(&router_timer, K_MSEC(new_timer));
705+
if (new_delay == UINT32_MAX) {
706+
k_delayed_work_cancel(&router_timer);
707+
} else {
708+
k_delayed_work_submit(&router_timer, K_MSEC(new_delay));
697709
}
698710
}
699711

@@ -707,10 +719,9 @@ static void iface_router_expired(struct k_work *work)
707719

708720
SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&active_router_timers,
709721
router, next, node) {
722+
int32_t ends = iface_router_ends(router, current_time);
710723

711-
if ((int32_t)(router->life_start +
712-
(MSEC_PER_SEC * router->lifetime) -
713-
current_time) > 0) {
724+
if (ends > 0) {
714725
/* We have to loop on all active routers as their
715726
* lifetime differ from each other.
716727
*/
@@ -724,7 +735,7 @@ static void iface_router_expired(struct k_work *work)
724735
router->is_used = false;
725736
}
726737

727-
iface_router_run_timer(current_time);
738+
iface_router_update_timer(current_time);
728739
}
729740

730741
static struct net_if_router *iface_router_add(struct net_if *iface,
@@ -752,7 +763,7 @@ static struct net_if_router *iface_router_add(struct net_if *iface,
752763
sys_slist_append(&active_router_timers,
753764
&routers[i].node);
754765

755-
iface_router_run_timer(routers[i].life_start);
766+
iface_router_update_timer(routers[i].life_start);
756767
} else {
757768
routers[i].is_default = false;
758769
routers[i].is_infinite = true;
@@ -805,7 +816,7 @@ static bool iface_router_rm(struct net_if_router *router)
805816

806817
/* We recompute the timer if only the router was time limited */
807818
if (sys_slist_find_and_remove(&active_router_timers, &router->node)) {
808-
iface_router_run_timer(k_uptime_get_32());
819+
iface_router_update_timer(k_uptime_get_32());
809820
}
810821

811822
router->is_used = false;
@@ -2139,7 +2150,7 @@ void net_if_ipv6_router_update_lifetime(struct net_if_router *router,
21392150
router->life_start = k_uptime_get_32();
21402151
router->lifetime = lifetime;
21412152

2142-
iface_router_run_timer(router->life_start);
2153+
iface_router_update_timer(router->life_start);
21432154
}
21442155

21452156
struct net_if_router *net_if_ipv6_router_add(struct net_if *iface,

0 commit comments

Comments
 (0)