Skip to content

Commit a9beb7e

Browse files
edumazetkuba-moo
authored andcommitted
neighbour: fix various data-races
1) tbl->gc_thresh1, tbl->gc_thresh2, tbl->gc_thresh3 and tbl->gc_interval can be written from sysfs. 2) tbl->last_flush is read locklessly from neigh_alloc() 3) tbl->proxy_queue.qlen is read locklessly from neightbl_fill_info() 4) neightbl_fill_info() reads cpu stats that can be changed concurrently. Fixes: c7fb64d ("[NETLINK]: Neighbour table configuration and statistics via rtnetlink") Signed-off-by: Eric Dumazet <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 72bf4f1 commit a9beb7e

File tree

1 file changed

+35
-32
lines changed

1 file changed

+35
-32
lines changed

net/core/neighbour.c

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,8 @@ bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl)
251251

252252
static int neigh_forced_gc(struct neigh_table *tbl)
253253
{
254-
int max_clean = atomic_read(&tbl->gc_entries) - tbl->gc_thresh2;
254+
int max_clean = atomic_read(&tbl->gc_entries) -
255+
READ_ONCE(tbl->gc_thresh2);
255256
unsigned long tref = jiffies - 5 * HZ;
256257
struct neighbour *n, *tmp;
257258
int shrunk = 0;
@@ -280,7 +281,7 @@ static int neigh_forced_gc(struct neigh_table *tbl)
280281
}
281282
}
282283

283-
tbl->last_flush = jiffies;
284+
WRITE_ONCE(tbl->last_flush, jiffies);
284285

285286
write_unlock_bh(&tbl->lock);
286287

@@ -464,17 +465,17 @@ static struct neighbour *neigh_alloc(struct neigh_table *tbl,
464465
{
465466
struct neighbour *n = NULL;
466467
unsigned long now = jiffies;
467-
int entries;
468+
int entries, gc_thresh3;
468469

469470
if (exempt_from_gc)
470471
goto do_alloc;
471472

472473
entries = atomic_inc_return(&tbl->gc_entries) - 1;
473-
if (entries >= tbl->gc_thresh3 ||
474-
(entries >= tbl->gc_thresh2 &&
475-
time_after(now, tbl->last_flush + 5 * HZ))) {
476-
if (!neigh_forced_gc(tbl) &&
477-
entries >= tbl->gc_thresh3) {
474+
gc_thresh3 = READ_ONCE(tbl->gc_thresh3);
475+
if (entries >= gc_thresh3 ||
476+
(entries >= READ_ONCE(tbl->gc_thresh2) &&
477+
time_after(now, READ_ONCE(tbl->last_flush) + 5 * HZ))) {
478+
if (!neigh_forced_gc(tbl) && entries >= gc_thresh3) {
478479
net_info_ratelimited("%s: neighbor table overflow!\n",
479480
tbl->id);
480481
NEIGH_CACHE_STAT_INC(tbl, table_fulls);
@@ -955,13 +956,14 @@ static void neigh_periodic_work(struct work_struct *work)
955956

956957
if (time_after(jiffies, tbl->last_rand + 300 * HZ)) {
957958
struct neigh_parms *p;
958-
tbl->last_rand = jiffies;
959+
960+
WRITE_ONCE(tbl->last_rand, jiffies);
959961
list_for_each_entry(p, &tbl->parms_list, list)
960962
p->reachable_time =
961963
neigh_rand_reach_time(NEIGH_VAR(p, BASE_REACHABLE_TIME));
962964
}
963965

964-
if (atomic_read(&tbl->entries) < tbl->gc_thresh1)
966+
if (atomic_read(&tbl->entries) < READ_ONCE(tbl->gc_thresh1))
965967
goto out;
966968

967969
for (i = 0 ; i < (1 << nht->hash_shift); i++) {
@@ -2167,23 +2169,24 @@ static int neightbl_fill_info(struct sk_buff *skb, struct neigh_table *tbl,
21672169
ndtmsg->ndtm_pad2 = 0;
21682170

21692171
if (nla_put_string(skb, NDTA_NAME, tbl->id) ||
2170-
nla_put_msecs(skb, NDTA_GC_INTERVAL, tbl->gc_interval, NDTA_PAD) ||
2171-
nla_put_u32(skb, NDTA_THRESH1, tbl->gc_thresh1) ||
2172-
nla_put_u32(skb, NDTA_THRESH2, tbl->gc_thresh2) ||
2173-
nla_put_u32(skb, NDTA_THRESH3, tbl->gc_thresh3))
2172+
nla_put_msecs(skb, NDTA_GC_INTERVAL, READ_ONCE(tbl->gc_interval),
2173+
NDTA_PAD) ||
2174+
nla_put_u32(skb, NDTA_THRESH1, READ_ONCE(tbl->gc_thresh1)) ||
2175+
nla_put_u32(skb, NDTA_THRESH2, READ_ONCE(tbl->gc_thresh2)) ||
2176+
nla_put_u32(skb, NDTA_THRESH3, READ_ONCE(tbl->gc_thresh3)))
21742177
goto nla_put_failure;
21752178
{
21762179
unsigned long now = jiffies;
2177-
long flush_delta = now - tbl->last_flush;
2178-
long rand_delta = now - tbl->last_rand;
2180+
long flush_delta = now - READ_ONCE(tbl->last_flush);
2181+
long rand_delta = now - READ_ONCE(tbl->last_rand);
21792182
struct neigh_hash_table *nht;
21802183
struct ndt_config ndc = {
21812184
.ndtc_key_len = tbl->key_len,
21822185
.ndtc_entry_size = tbl->entry_size,
21832186
.ndtc_entries = atomic_read(&tbl->entries),
21842187
.ndtc_last_flush = jiffies_to_msecs(flush_delta),
21852188
.ndtc_last_rand = jiffies_to_msecs(rand_delta),
2186-
.ndtc_proxy_qlen = tbl->proxy_queue.qlen,
2189+
.ndtc_proxy_qlen = READ_ONCE(tbl->proxy_queue.qlen),
21872190
};
21882191

21892192
rcu_read_lock();
@@ -2206,17 +2209,17 @@ static int neightbl_fill_info(struct sk_buff *skb, struct neigh_table *tbl,
22062209
struct neigh_statistics *st;
22072210

22082211
st = per_cpu_ptr(tbl->stats, cpu);
2209-
ndst.ndts_allocs += st->allocs;
2210-
ndst.ndts_destroys += st->destroys;
2211-
ndst.ndts_hash_grows += st->hash_grows;
2212-
ndst.ndts_res_failed += st->res_failed;
2213-
ndst.ndts_lookups += st->lookups;
2214-
ndst.ndts_hits += st->hits;
2215-
ndst.ndts_rcv_probes_mcast += st->rcv_probes_mcast;
2216-
ndst.ndts_rcv_probes_ucast += st->rcv_probes_ucast;
2217-
ndst.ndts_periodic_gc_runs += st->periodic_gc_runs;
2218-
ndst.ndts_forced_gc_runs += st->forced_gc_runs;
2219-
ndst.ndts_table_fulls += st->table_fulls;
2212+
ndst.ndts_allocs += READ_ONCE(st->allocs);
2213+
ndst.ndts_destroys += READ_ONCE(st->destroys);
2214+
ndst.ndts_hash_grows += READ_ONCE(st->hash_grows);
2215+
ndst.ndts_res_failed += READ_ONCE(st->res_failed);
2216+
ndst.ndts_lookups += READ_ONCE(st->lookups);
2217+
ndst.ndts_hits += READ_ONCE(st->hits);
2218+
ndst.ndts_rcv_probes_mcast += READ_ONCE(st->rcv_probes_mcast);
2219+
ndst.ndts_rcv_probes_ucast += READ_ONCE(st->rcv_probes_ucast);
2220+
ndst.ndts_periodic_gc_runs += READ_ONCE(st->periodic_gc_runs);
2221+
ndst.ndts_forced_gc_runs += READ_ONCE(st->forced_gc_runs);
2222+
ndst.ndts_table_fulls += READ_ONCE(st->table_fulls);
22202223
}
22212224

22222225
if (nla_put_64bit(skb, NDTA_STATS, sizeof(ndst), &ndst,
@@ -2445,16 +2448,16 @@ static int neightbl_set(struct sk_buff *skb, struct nlmsghdr *nlh,
24452448
goto errout_tbl_lock;
24462449

24472450
if (tb[NDTA_THRESH1])
2448-
tbl->gc_thresh1 = nla_get_u32(tb[NDTA_THRESH1]);
2451+
WRITE_ONCE(tbl->gc_thresh1, nla_get_u32(tb[NDTA_THRESH1]));
24492452

24502453
if (tb[NDTA_THRESH2])
2451-
tbl->gc_thresh2 = nla_get_u32(tb[NDTA_THRESH2]);
2454+
WRITE_ONCE(tbl->gc_thresh2, nla_get_u32(tb[NDTA_THRESH2]));
24522455

24532456
if (tb[NDTA_THRESH3])
2454-
tbl->gc_thresh3 = nla_get_u32(tb[NDTA_THRESH3]);
2457+
WRITE_ONCE(tbl->gc_thresh3, nla_get_u32(tb[NDTA_THRESH3]));
24552458

24562459
if (tb[NDTA_GC_INTERVAL])
2457-
tbl->gc_interval = nla_get_msecs(tb[NDTA_GC_INTERVAL]);
2460+
WRITE_ONCE(tbl->gc_interval, nla_get_msecs(tb[NDTA_GC_INTERVAL]));
24582461

24592462
err = 0;
24602463

0 commit comments

Comments
 (0)