Skip to content

Commit 904eaf9

Browse files
klassertklassert
committed
Merge branch 'xfrm-fix-most-sparse-warnings'
Sabrina Dubroca says: ==================== xfrm: fix most sparse warnings This series fixes most of the sparse warnings currently reported about RCU pointers for files under net/xfrm. There's no actual bug in the current code, we only need to use the correct helpers in each context. ==================== Signed-off-by: Steffen Klassert <[email protected]>
2 parents 0b352f8 + d87f8bc commit 904eaf9

5 files changed

Lines changed: 90 additions & 67 deletions

File tree

‎include/net/netns/xfrm.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ struct netns_xfrm {
5959
struct list_head inexact_bins;
6060

6161

62-
struct sock *nlsk;
62+
struct sock __rcu *nlsk;
6363
struct sock *nlsk_stash;
6464

6565
u32 sysctl_aevent_etime;

‎net/xfrm/xfrm_input.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ int xfrm_input_unregister_afinfo(const struct xfrm_input_afinfo *afinfo)
7575

7676
spin_lock_bh(&xfrm_input_afinfo_lock);
7777
if (likely(xfrm_input_afinfo[afinfo->is_ipip][afinfo->family])) {
78-
if (unlikely(xfrm_input_afinfo[afinfo->is_ipip][afinfo->family] != afinfo))
78+
const struct xfrm_input_afinfo *cur;
79+
80+
cur = rcu_access_pointer(xfrm_input_afinfo[afinfo->is_ipip][afinfo->family]);
81+
if (unlikely(cur != afinfo))
7982
err = -EINVAL;
8083
else
8184
RCU_INIT_POINTER(xfrm_input_afinfo[afinfo->is_ipip][afinfo->family], NULL);

‎net/xfrm/xfrm_policy.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4156,7 +4156,7 @@ void xfrm_policy_unregister_afinfo(const struct xfrm_policy_afinfo *afinfo)
41564156
int i;
41574157

41584158
for (i = 0; i < ARRAY_SIZE(xfrm_policy_afinfo); i++) {
4159-
if (xfrm_policy_afinfo[i] != afinfo)
4159+
if (rcu_access_pointer(xfrm_policy_afinfo[i]) != afinfo)
41604160
continue;
41614161
RCU_INIT_POINTER(xfrm_policy_afinfo[i], NULL);
41624162
break;
@@ -4242,7 +4242,7 @@ static int __net_init xfrm_policy_init(struct net *net)
42424242
net->xfrm.policy_count[XFRM_POLICY_MAX + dir] = 0;
42434243

42444244
htab = &net->xfrm.policy_bydst[dir];
4245-
htab->table = xfrm_hash_alloc(sz);
4245+
rcu_assign_pointer(htab->table, xfrm_hash_alloc(sz));
42464246
if (!htab->table)
42474247
goto out_bydst;
42484248
htab->hmask = hmask;
@@ -4269,7 +4269,7 @@ static int __net_init xfrm_policy_init(struct net *net)
42694269
struct xfrm_policy_hash *htab;
42704270

42714271
htab = &net->xfrm.policy_bydst[dir];
4272-
xfrm_hash_free(htab->table, sz);
4272+
xfrm_hash_free(rcu_dereference_protected(htab->table, true), sz);
42734273
}
42744274
xfrm_hash_free(net->xfrm.policy_byidx, sz);
42754275
out_byidx:
@@ -4295,8 +4295,8 @@ static void xfrm_policy_fini(struct net *net)
42954295

42964296
htab = &net->xfrm.policy_bydst[dir];
42974297
sz = (htab->hmask + 1) * sizeof(struct hlist_head);
4298-
WARN_ON(!hlist_empty(htab->table));
4299-
xfrm_hash_free(htab->table, sz);
4298+
WARN_ON(!hlist_empty(rcu_dereference_protected(htab->table, true)));
4299+
xfrm_hash_free(rcu_dereference_protected(htab->table, true), sz);
43004300
}
43014301

43024302
sz = (net->xfrm.policy_idx_hmask + 1) * sizeof(struct hlist_head);

‎net/xfrm/xfrm_state.c‎

Lines changed: 63 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ static DECLARE_WORK(xfrm_state_gc_work, xfrm_state_gc_task);
5353
static HLIST_HEAD(xfrm_state_gc_list);
5454
static HLIST_HEAD(xfrm_state_dev_gc_list);
5555

56-
static inline bool xfrm_state_hold_rcu(struct xfrm_state __rcu *x)
56+
static inline bool xfrm_state_hold_rcu(struct xfrm_state *x)
5757
{
5858
return refcount_inc_not_zero(&x->refcnt);
5959
}
@@ -870,7 +870,7 @@ xfrm_state_flush_secctx_check(struct net *net, u8 proto, bool task_valid)
870870
for (i = 0; i <= net->xfrm.state_hmask; i++) {
871871
struct xfrm_state *x;
872872

873-
hlist_for_each_entry(x, net->xfrm.state_bydst+i, bydst) {
873+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + i, bydst) {
874874
if (xfrm_id_proto_match(x->id.proto, proto) &&
875875
(err = security_xfrm_state_delete(x)) != 0) {
876876
xfrm_audit_state_delete(x, 0, task_valid);
@@ -891,7 +891,7 @@ xfrm_dev_state_flush_secctx_check(struct net *net, struct net_device *dev, bool
891891
struct xfrm_state *x;
892892
struct xfrm_dev_offload *xso;
893893

894-
hlist_for_each_entry(x, net->xfrm.state_bydst+i, bydst) {
894+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + i, bydst) {
895895
xso = &x->xso;
896896

897897
if (xso->dev == dev &&
@@ -931,7 +931,7 @@ int xfrm_state_flush(struct net *net, u8 proto, bool task_valid)
931931
for (i = 0; i <= net->xfrm.state_hmask; i++) {
932932
struct xfrm_state *x;
933933
restart:
934-
hlist_for_each_entry(x, net->xfrm.state_bydst+i, bydst) {
934+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + i, bydst) {
935935
if (!xfrm_state_kern(x) &&
936936
xfrm_id_proto_match(x->id.proto, proto)) {
937937
xfrm_state_hold(x);
@@ -973,7 +973,7 @@ int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_vali
973973
err = -ESRCH;
974974
for (i = 0; i <= net->xfrm.state_hmask; i++) {
975975
restart:
976-
hlist_for_each_entry(x, net->xfrm.state_bydst+i, bydst) {
976+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + i, bydst) {
977977
xso = &x->xso;
978978

979979
if (!xfrm_state_kern(x) && xso->dev == dev) {
@@ -1563,23 +1563,23 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
15631563
list_add(&x->km.all, &net->xfrm.state_all);
15641564
h = xfrm_dst_hash(net, daddr, saddr, tmpl->reqid, encap_family);
15651565
XFRM_STATE_INSERT(bydst, &x->bydst,
1566-
net->xfrm.state_bydst + h,
1566+
xfrm_state_deref_prot(net->xfrm.state_bydst, net) + h,
15671567
x->xso.type);
15681568
h = xfrm_src_hash(net, daddr, saddr, encap_family);
15691569
XFRM_STATE_INSERT(bysrc, &x->bysrc,
1570-
net->xfrm.state_bysrc + h,
1570+
xfrm_state_deref_prot(net->xfrm.state_bysrc, net) + h,
15711571
x->xso.type);
15721572
INIT_HLIST_NODE(&x->state_cache);
15731573
if (x->id.spi) {
15741574
h = xfrm_spi_hash(net, &x->id.daddr, x->id.spi, x->id.proto, encap_family);
15751575
XFRM_STATE_INSERT(byspi, &x->byspi,
1576-
net->xfrm.state_byspi + h,
1576+
xfrm_state_deref_prot(net->xfrm.state_byspi, net) + h,
15771577
x->xso.type);
15781578
}
15791579
if (x->km.seq) {
15801580
h = xfrm_seq_hash(net, x->km.seq);
15811581
XFRM_STATE_INSERT(byseq, &x->byseq,
1582-
net->xfrm.state_byseq + h,
1582+
xfrm_state_deref_prot(net->xfrm.state_byseq, net) + h,
15831583
x->xso.type);
15841584
}
15851585
x->lft.hard_add_expires_seconds = net->xfrm.sysctl_acq_expires;
@@ -1652,7 +1652,7 @@ xfrm_stateonly_find(struct net *net, u32 mark, u32 if_id,
16521652

16531653
spin_lock_bh(&net->xfrm.xfrm_state_lock);
16541654
h = xfrm_dst_hash(net, daddr, saddr, reqid, family);
1655-
hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) {
1655+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + h, bydst) {
16561656
if (x->props.family == family &&
16571657
x->props.reqid == reqid &&
16581658
(mark & x->mark.m) == x->mark.v &&
@@ -1703,18 +1703,12 @@ static struct xfrm_state *xfrm_state_lookup_spi_proto(struct net *net, __be32 sp
17031703
struct xfrm_state *x;
17041704
unsigned int i;
17051705

1706-
rcu_read_lock();
17071706
for (i = 0; i <= net->xfrm.state_hmask; i++) {
1708-
hlist_for_each_entry_rcu(x, &net->xfrm.state_byspi[i], byspi) {
1709-
if (x->id.spi == spi && x->id.proto == proto) {
1710-
if (!xfrm_state_hold_rcu(x))
1711-
continue;
1712-
rcu_read_unlock();
1707+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_byspi, net) + i, byspi) {
1708+
if (x->id.spi == spi && x->id.proto == proto)
17131709
return x;
1714-
}
17151710
}
17161711
}
1717-
rcu_read_unlock();
17181712
return NULL;
17191713
}
17201714

@@ -1730,25 +1724,29 @@ static void __xfrm_state_insert(struct xfrm_state *x)
17301724

17311725
h = xfrm_dst_hash(net, &x->id.daddr, &x->props.saddr,
17321726
x->props.reqid, x->props.family);
1733-
XFRM_STATE_INSERT(bydst, &x->bydst, net->xfrm.state_bydst + h,
1727+
XFRM_STATE_INSERT(bydst, &x->bydst,
1728+
xfrm_state_deref_prot(net->xfrm.state_bydst, net) + h,
17341729
x->xso.type);
17351730

17361731
h = xfrm_src_hash(net, &x->id.daddr, &x->props.saddr, x->props.family);
1737-
XFRM_STATE_INSERT(bysrc, &x->bysrc, net->xfrm.state_bysrc + h,
1732+
XFRM_STATE_INSERT(bysrc, &x->bysrc,
1733+
xfrm_state_deref_prot(net->xfrm.state_bysrc, net) + h,
17381734
x->xso.type);
17391735

17401736
if (x->id.spi) {
17411737
h = xfrm_spi_hash(net, &x->id.daddr, x->id.spi, x->id.proto,
17421738
x->props.family);
17431739

1744-
XFRM_STATE_INSERT(byspi, &x->byspi, net->xfrm.state_byspi + h,
1740+
XFRM_STATE_INSERT(byspi, &x->byspi,
1741+
xfrm_state_deref_prot(net->xfrm.state_byspi, net) + h,
17451742
x->xso.type);
17461743
}
17471744

17481745
if (x->km.seq) {
17491746
h = xfrm_seq_hash(net, x->km.seq);
17501747

1751-
XFRM_STATE_INSERT(byseq, &x->byseq, net->xfrm.state_byseq + h,
1748+
XFRM_STATE_INSERT(byseq, &x->byseq,
1749+
xfrm_state_deref_prot(net->xfrm.state_byseq, net) + h,
17521750
x->xso.type);
17531751
}
17541752

@@ -1775,7 +1773,7 @@ static void __xfrm_state_bump_genids(struct xfrm_state *xnew)
17751773
u32 cpu_id = xnew->pcpu_num;
17761774

17771775
h = xfrm_dst_hash(net, &xnew->id.daddr, &xnew->props.saddr, reqid, family);
1778-
hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) {
1776+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + h, bydst) {
17791777
if (x->props.family == family &&
17801778
x->props.reqid == reqid &&
17811779
x->if_id == if_id &&
@@ -1811,7 +1809,7 @@ static struct xfrm_state *__find_acq_core(struct net *net,
18111809
struct xfrm_state *x;
18121810
u32 mark = m->v & m->m;
18131811

1814-
hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) {
1812+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + h, bydst) {
18151813
if (x->props.reqid != reqid ||
18161814
x->props.mode != mode ||
18171815
x->props.family != family ||
@@ -1868,10 +1866,12 @@ static struct xfrm_state *__find_acq_core(struct net *net,
18681866
ktime_set(net->xfrm.sysctl_acq_expires, 0),
18691867
HRTIMER_MODE_REL_SOFT);
18701868
list_add(&x->km.all, &net->xfrm.state_all);
1871-
XFRM_STATE_INSERT(bydst, &x->bydst, net->xfrm.state_bydst + h,
1869+
XFRM_STATE_INSERT(bydst, &x->bydst,
1870+
xfrm_state_deref_prot(net->xfrm.state_bydst, net) + h,
18721871
x->xso.type);
18731872
h = xfrm_src_hash(net, daddr, saddr, family);
1874-
XFRM_STATE_INSERT(bysrc, &x->bysrc, net->xfrm.state_bysrc + h,
1873+
XFRM_STATE_INSERT(bysrc, &x->bysrc,
1874+
xfrm_state_deref_prot(net->xfrm.state_bysrc, net) + h,
18751875
x->xso.type);
18761876

18771877
net->xfrm.state_num++;
@@ -2091,7 +2091,7 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
20912091
if (m->reqid) {
20922092
h = xfrm_dst_hash(net, &m->old_daddr, &m->old_saddr,
20932093
m->reqid, m->old_family);
2094-
hlist_for_each_entry(x, net->xfrm.state_bydst+h, bydst) {
2094+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + h, bydst) {
20952095
if (x->props.mode != m->mode ||
20962096
x->id.proto != m->proto)
20972097
continue;
@@ -2110,7 +2110,7 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
21102110
} else {
21112111
h = xfrm_src_hash(net, &m->old_daddr, &m->old_saddr,
21122112
m->old_family);
2113-
hlist_for_each_entry(x, net->xfrm.state_bysrc+h, bysrc) {
2113+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bysrc, net) + h, bysrc) {
21142114
if (x->props.mode != m->mode ||
21152115
x->id.proto != m->proto)
21162116
continue;
@@ -2313,7 +2313,7 @@ void xfrm_state_update_stats(struct net *net)
23132313

23142314
spin_lock_bh(&net->xfrm.xfrm_state_lock);
23152315
for (i = 0; i <= net->xfrm.state_hmask; i++) {
2316-
hlist_for_each_entry(x, net->xfrm.state_bydst + i, bydst)
2316+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_bydst, net) + i, bydst)
23172317
xfrm_dev_state_update_stats(x);
23182318
}
23192319
spin_unlock_bh(&net->xfrm.xfrm_state_lock);
@@ -2504,7 +2504,7 @@ static struct xfrm_state *__xfrm_find_acq_byseq(struct net *net, u32 mark, u32 s
25042504
unsigned int h = xfrm_seq_hash(net, seq);
25052505
struct xfrm_state *x;
25062506

2507-
hlist_for_each_entry_rcu(x, net->xfrm.state_byseq + h, byseq) {
2507+
hlist_for_each_entry(x, xfrm_state_deref_prot(net->xfrm.state_byseq, net) + h, byseq) {
25082508
if (x->km.seq == seq &&
25092509
(mark & x->mark.m) == x->mark.v &&
25102510
x->pcpu_num == pcpu_num &&
@@ -2603,12 +2603,13 @@ int xfrm_alloc_spi(struct xfrm_state *x, u32 low, u32 high,
26032603
if (!x0) {
26042604
x->id.spi = newspi;
26052605
h = xfrm_spi_hash(net, &x->id.daddr, newspi, x->id.proto, x->props.family);
2606-
XFRM_STATE_INSERT(byspi, &x->byspi, net->xfrm.state_byspi + h, x->xso.type);
2606+
XFRM_STATE_INSERT(byspi, &x->byspi,
2607+
xfrm_state_deref_prot(net->xfrm.state_byspi, net) + h,
2608+
x->xso.type);
26072609
spin_unlock_bh(&net->xfrm.xfrm_state_lock);
26082610
err = 0;
26092611
goto unlock;
26102612
}
2611-
xfrm_state_put(x0);
26122613
spin_unlock_bh(&net->xfrm.xfrm_state_lock);
26132614

26142615
next:
@@ -3259,6 +3260,7 @@ EXPORT_SYMBOL(xfrm_init_state);
32593260

32603261
int __net_init xfrm_state_init(struct net *net)
32613262
{
3263+
struct hlist_head *ndst, *nsrc, *nspi, *nseq;
32623264
unsigned int sz;
32633265

32643266
if (net_eq(net, &init_net))
@@ -3269,18 +3271,25 @@ int __net_init xfrm_state_init(struct net *net)
32693271

32703272
sz = sizeof(struct hlist_head) * 8;
32713273

3272-
net->xfrm.state_bydst = xfrm_hash_alloc(sz);
3273-
if (!net->xfrm.state_bydst)
3274+
ndst = xfrm_hash_alloc(sz);
3275+
if (!ndst)
32743276
goto out_bydst;
3275-
net->xfrm.state_bysrc = xfrm_hash_alloc(sz);
3276-
if (!net->xfrm.state_bysrc)
3277+
rcu_assign_pointer(net->xfrm.state_bydst, ndst);
3278+
3279+
nsrc = xfrm_hash_alloc(sz);
3280+
if (!nsrc)
32773281
goto out_bysrc;
3278-
net->xfrm.state_byspi = xfrm_hash_alloc(sz);
3279-
if (!net->xfrm.state_byspi)
3282+
rcu_assign_pointer(net->xfrm.state_bysrc, nsrc);
3283+
3284+
nspi = xfrm_hash_alloc(sz);
3285+
if (!nspi)
32803286
goto out_byspi;
3281-
net->xfrm.state_byseq = xfrm_hash_alloc(sz);
3282-
if (!net->xfrm.state_byseq)
3287+
rcu_assign_pointer(net->xfrm.state_byspi, nspi);
3288+
3289+
nseq = xfrm_hash_alloc(sz);
3290+
if (!nseq)
32833291
goto out_byseq;
3292+
rcu_assign_pointer(net->xfrm.state_byseq, nseq);
32843293

32853294
net->xfrm.state_cache_input = alloc_percpu(struct hlist_head);
32863295
if (!net->xfrm.state_cache_input)
@@ -3296,17 +3305,19 @@ int __net_init xfrm_state_init(struct net *net)
32963305
return 0;
32973306

32983307
out_state_cache_input:
3299-
xfrm_hash_free(net->xfrm.state_byseq, sz);
3308+
xfrm_hash_free(nseq, sz);
33003309
out_byseq:
3301-
xfrm_hash_free(net->xfrm.state_byspi, sz);
3310+
xfrm_hash_free(nspi, sz);
33023311
out_byspi:
3303-
xfrm_hash_free(net->xfrm.state_bysrc, sz);
3312+
xfrm_hash_free(nsrc, sz);
33043313
out_bysrc:
3305-
xfrm_hash_free(net->xfrm.state_bydst, sz);
3314+
xfrm_hash_free(ndst, sz);
33063315
out_bydst:
33073316
return -ENOMEM;
33083317
}
33093318

3319+
#define xfrm_state_deref_netexit(table) \
3320+
rcu_dereference_protected((table), true /* netns is going away */)
33103321
void xfrm_state_fini(struct net *net)
33113322
{
33123323
unsigned int sz;
@@ -3319,17 +3330,17 @@ void xfrm_state_fini(struct net *net)
33193330
WARN_ON(!list_empty(&net->xfrm.state_all));
33203331

33213332
for (i = 0; i <= net->xfrm.state_hmask; i++) {
3322-
WARN_ON(!hlist_empty(net->xfrm.state_byseq + i));
3323-
WARN_ON(!hlist_empty(net->xfrm.state_byspi + i));
3324-
WARN_ON(!hlist_empty(net->xfrm.state_bysrc + i));
3325-
WARN_ON(!hlist_empty(net->xfrm.state_bydst + i));
3333+
WARN_ON(!hlist_empty(xfrm_state_deref_netexit(net->xfrm.state_byseq) + i));
3334+
WARN_ON(!hlist_empty(xfrm_state_deref_netexit(net->xfrm.state_byspi) + i));
3335+
WARN_ON(!hlist_empty(xfrm_state_deref_netexit(net->xfrm.state_bysrc) + i));
3336+
WARN_ON(!hlist_empty(xfrm_state_deref_netexit(net->xfrm.state_bydst) + i));
33263337
}
33273338

33283339
sz = (net->xfrm.state_hmask + 1) * sizeof(struct hlist_head);
3329-
xfrm_hash_free(net->xfrm.state_byseq, sz);
3330-
xfrm_hash_free(net->xfrm.state_byspi, sz);
3331-
xfrm_hash_free(net->xfrm.state_bysrc, sz);
3332-
xfrm_hash_free(net->xfrm.state_bydst, sz);
3340+
xfrm_hash_free(xfrm_state_deref_netexit(net->xfrm.state_byseq), sz);
3341+
xfrm_hash_free(xfrm_state_deref_netexit(net->xfrm.state_byspi), sz);
3342+
xfrm_hash_free(xfrm_state_deref_netexit(net->xfrm.state_bysrc), sz);
3343+
xfrm_hash_free(xfrm_state_deref_netexit(net->xfrm.state_bydst), sz);
33333344
free_percpu(net->xfrm.state_cache_input);
33343345
}
33353346

0 commit comments

Comments
 (0)