Skip to content

Commit c1d2ecd

Browse files
Julian Anastasovdavem330
authored andcommitted
neigh: make sure used and confirmed times are valid
Entries can linger in cache without timer for days, thanks to the gc_thresh1 limit. As result, without traffic, the confirmed time can be outdated and to appear to be in the future. Later, on traffic, NUD_STALE entries can switch to NUD_DELAY and start the timer which can see the invalid confirmed time and wrongly switch to NUD_REACHABLE state instead of NUD_PROBE. As result, timer is set many days in the future. This is more visible on 32-bit platforms, with higher HZ value. Why this is a problem? While we expect unused entries to expire, such entries stay in REACHABLE state for too long, locked in cache. They are not expired normally, only when cache is full. Problem and the wrong state change reported by Zhang Changzhong: 172.16.1.18 dev bond0 lladdr 0a:0e:0f:01:12:01 ref 1 used 350521/15994171/350520 probes 4 REACHABLE 350520 seconds have elapsed since this entry was last updated, but it is still in the REACHABLE state (base_reachable_time_ms is 30000), preventing lladdr from being updated through probe. Fix it by ensuring timer is started with valid used/confirmed times. Considering the valid time range is LONG_MAX jiffies, we try not to go too much in the past while we are in DELAY/PROBE state. There are also places that need used/updated times to be validated while timer is not running. Reported-by: Zhang Changzhong <[email protected]> Signed-off-by: Julian Anastasov <[email protected]> Tested-by: Zhang Changzhong <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent a05e7a6 commit c1d2ecd

File tree

1 file changed

+15
-3
lines changed

1 file changed

+15
-3
lines changed

net/core/neighbour.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ static int neigh_forced_gc(struct neigh_table *tbl)
269269
(n->nud_state == NUD_NOARP) ||
270270
(tbl->is_multicast &&
271271
tbl->is_multicast(n->primary_key)) ||
272-
time_after(tref, n->updated))
272+
!time_in_range(n->updated, tref, jiffies))
273273
remove = true;
274274
write_unlock(&n->lock);
275275

@@ -289,7 +289,17 @@ static int neigh_forced_gc(struct neigh_table *tbl)
289289

290290
static void neigh_add_timer(struct neighbour *n, unsigned long when)
291291
{
292+
/* Use safe distance from the jiffies - LONG_MAX point while timer
293+
* is running in DELAY/PROBE state but still show to user space
294+
* large times in the past.
295+
*/
296+
unsigned long mint = jiffies - (LONG_MAX - 86400 * HZ);
297+
292298
neigh_hold(n);
299+
if (!time_in_range(n->confirmed, mint, jiffies))
300+
n->confirmed = mint;
301+
if (time_before(n->used, n->confirmed))
302+
n->used = n->confirmed;
293303
if (unlikely(mod_timer(&n->timer, when))) {
294304
printk("NEIGH: BUG, double timer add, state is %x\n",
295305
n->nud_state);
@@ -1001,12 +1011,14 @@ static void neigh_periodic_work(struct work_struct *work)
10011011
goto next_elt;
10021012
}
10031013

1004-
if (time_before(n->used, n->confirmed))
1014+
if (time_before(n->used, n->confirmed) &&
1015+
time_is_before_eq_jiffies(n->confirmed))
10051016
n->used = n->confirmed;
10061017

10071018
if (refcount_read(&n->refcnt) == 1 &&
10081019
(state == NUD_FAILED ||
1009-
time_after(jiffies, n->used + NEIGH_VAR(n->parms, GC_STALETIME)))) {
1020+
!time_in_range_open(jiffies, n->used,
1021+
n->used + NEIGH_VAR(n->parms, GC_STALETIME)))) {
10101022
*np = n->next;
10111023
neigh_mark_dead(n);
10121024
write_unlock(&n->lock);

0 commit comments

Comments
 (0)