xfrm: annotate data-races around xfrm_policy_count[] and xfrm_policy_default[]

KCSAN reported a data race involving net->xfrm.policy_count access.

Add missing READ_ONCE()/WRITE_ONCE() annotations on
xfrm_policy_count and xfrm_policy_default.

Fixes: 2518c7c2b3d7 ("[XFRM]: Hash policies when non-prefixed.")
Reported-by: syzbot+d85ba1c732720b9a4097@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6a2b9e96.99669fcc.12a77b.0006.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 874409127e292197c17dbac4686efdd5ff56c185..35a7431293298a9eb7f82c8a865ca45e227e81e1 100644 (file)
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1250,8 +1250,8 @@ int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb,
 static inline bool __xfrm_check_nopolicy(struct net *net, struct sk_buff *skb,
                                         int dir)
 {
-       if (!net->xfrm.policy_count[dir] && !secpath_exists(skb))
-               return net->xfrm.policy_default[dir] == XFRM_USERPOLICY_ACCEPT;
+       if (!READ_ONCE(net->xfrm.policy_count[dir]) && !secpath_exists(skb))
+               return READ_ONCE(net->xfrm.policy_default[dir]) == XFRM_USERPOLICY_ACCEPT;
 
        return false;
 }
@@ -1351,8 +1351,8 @@ static inline int xfrm_route_forward(struct sk_buff *skb, unsigned short family)
 {
        struct net *net = dev_net(skb->dev);
 
-       if (!net->xfrm.policy_count[XFRM_POLICY_OUT] &&
-           net->xfrm.policy_default[XFRM_POLICY_OUT] == XFRM_USERPOLICY_ACCEPT)
+       if (!READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_OUT]) &&
+           READ_ONCE(net->xfrm.policy_default[XFRM_POLICY_OUT]) == XFRM_USERPOLICY_ACCEPT)
                return true;
 
        return (skb_dst(skb)->flags & DST_NOXFRM) ||

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 95954442569290719b9fdb7b0f9462d70b5d755e..1f4afd580105f97094c6ad8d10001912f67cc731 100644 (file)
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -685,7 +685,7 @@ static void xfrm_byidx_resize(struct net *net)
 
 static inline int xfrm_bydst_should_resize(struct net *net, int dir, int *total)
 {
-       unsigned int cnt = net->xfrm.policy_count[dir];
+       unsigned int cnt = READ_ONCE(net->xfrm.policy_count[dir]);
        unsigned int hmask = net->xfrm.policy_bydst[dir].hmask;
 
        if (total)
@@ -711,12 +711,12 @@ static inline int xfrm_byidx_should_resize(struct net *net, int total)
 
 void xfrm_spd_getinfo(struct net *net, struct xfrmk_spdinfo *si)
 {
-       si->incnt = net->xfrm.policy_count[XFRM_POLICY_IN];
-       si->outcnt = net->xfrm.policy_count[XFRM_POLICY_OUT];
-       si->fwdcnt = net->xfrm.policy_count[XFRM_POLICY_FWD];
-       si->inscnt = net->xfrm.policy_count[XFRM_POLICY_IN+XFRM_POLICY_MAX];
-       si->outscnt = net->xfrm.policy_count[XFRM_POLICY_OUT+XFRM_POLICY_MAX];
-       si->fwdscnt = net->xfrm.policy_count[XFRM_POLICY_FWD+XFRM_POLICY_MAX];
+       si->incnt = READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_IN]);
+       si->outcnt = READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_OUT]);
+       si->fwdcnt = READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_FWD]);
+       si->inscnt = READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_IN+XFRM_POLICY_MAX]);
+       si->outscnt = READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_OUT+XFRM_POLICY_MAX]);
+       si->fwdscnt = READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_FWD+XFRM_POLICY_MAX]);
        si->spdhcnt = net->xfrm.policy_idx_hmask;
        si->spdhmcnt = xfrm_policy_hashmax;
 }
@@ -2318,7 +2318,7 @@ static void __xfrm_policy_link(struct xfrm_policy *pol, int dir)
        }
 
        list_add(&pol->walk.all, &net->xfrm.policy_all);
-       net->xfrm.policy_count[dir]++;
+       WRITE_ONCE(net->xfrm.policy_count[dir], net->xfrm.policy_count[dir] + 1);
        xfrm_pol_hold(pol);
 }
 
@@ -2337,7 +2337,7 @@ static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
        }
 
        list_del_init(&pol->walk.all);
-       net->xfrm.policy_count[dir]--;
+       WRITE_ONCE(net->xfrm.policy_count[dir], net->xfrm.policy_count[dir] - 1);
 
        return pol;
 }
@@ -3222,7 +3222,7 @@ struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
 
                /* To accelerate a bit...  */
                if (!if_id && ((dst_orig->flags & DST_NOXFRM) ||
-                              !net->xfrm.policy_count[XFRM_POLICY_OUT]))
+                              !READ_ONCE(net->xfrm.policy_count[XFRM_POLICY_OUT])))
                        goto nopol;
 
                xdst = xfrm_bundle_lookup(net, fl, family, dir, &xflo, if_id);
@@ -3296,7 +3296,7 @@ ok:
 
 nopol:
        if ((!dst_orig->dev || !(dst_orig->dev->flags & IFF_LOOPBACK)) &&
-           net->xfrm.policy_default[dir] == XFRM_USERPOLICY_BLOCK) {
+           READ_ONCE(net->xfrm.policy_default[dir]) == XFRM_USERPOLICY_BLOCK) {
                err = -EPERM;
                goto error;
        }
@@ -3750,7 +3750,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
                const bool is_crypto_offload = sp &&
                        (xfrm_input_state(skb)->xso.type == XFRM_DEV_OFFLOAD_CRYPTO);
 
-               if (net->xfrm.policy_default[dir] == XFRM_USERPOLICY_BLOCK) {
+               if (READ_ONCE(net->xfrm.policy_default[dir]) == XFRM_USERPOLICY_BLOCK) {
                        XFRM_INC_STATS(net, LINUX_MIB_XFRMINNOPOLS);
                        return 0;
                }

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 3b1cf29bc402193c5824eac75d5505be2927a17e..61eb5de33b87eef42dfa4f5fddfe5effb9538a8e 100644 (file)
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2485,9 +2485,9 @@ static int xfrm_notify_userpolicy(struct net *net)
        }
 
        up = nlmsg_data(nlh);
-       up->in = net->xfrm.policy_default[XFRM_POLICY_IN];
-       up->fwd = net->xfrm.policy_default[XFRM_POLICY_FWD];
-       up->out = net->xfrm.policy_default[XFRM_POLICY_OUT];
+       up->in = READ_ONCE(net->xfrm.policy_default[XFRM_POLICY_IN]);
+       up->fwd = READ_ONCE(net->xfrm.policy_default[XFRM_POLICY_FWD]);
+       up->out = READ_ONCE(net->xfrm.policy_default[XFRM_POLICY_OUT]);
 
        nlmsg_end(skb, nlh);
 
@@ -2511,13 +2511,13 @@ static int xfrm_set_default(struct sk_buff *skb, struct nlmsghdr *nlh,
        struct xfrm_userpolicy_default *up = nlmsg_data(nlh);
 
        if (xfrm_userpolicy_is_valid(up->in))
-               net->xfrm.policy_default[XFRM_POLICY_IN] = up->in;
+               WRITE_ONCE(net->xfrm.policy_default[XFRM_POLICY_IN], up->in);
 
        if (xfrm_userpolicy_is_valid(up->fwd))
-               net->xfrm.policy_default[XFRM_POLICY_FWD] = up->fwd;
+               WRITE_ONCE(net->xfrm.policy_default[XFRM_POLICY_FWD], up->fwd);
 
        if (xfrm_userpolicy_is_valid(up->out))
-               net->xfrm.policy_default[XFRM_POLICY_OUT] = up->out;
+               WRITE_ONCE(net->xfrm.policy_default[XFRM_POLICY_OUT], up->out);
 
        rt_genid_bump_all(net);
 
@@ -2547,9 +2547,9 @@ static int xfrm_get_default(struct sk_buff *skb, struct nlmsghdr *nlh,
        }
 
        r_up = nlmsg_data(r_nlh);
-       r_up->in = net->xfrm.policy_default[XFRM_POLICY_IN];
-       r_up->fwd = net->xfrm.policy_default[XFRM_POLICY_FWD];
-       r_up->out = net->xfrm.policy_default[XFRM_POLICY_OUT];
+       r_up->in = READ_ONCE(net->xfrm.policy_default[XFRM_POLICY_IN]);
+       r_up->fwd = READ_ONCE(net->xfrm.policy_default[XFRM_POLICY_FWD]);
+       r_up->out = READ_ONCE(net->xfrm.policy_default[XFRM_POLICY_OUT]);
        nlmsg_end(r_skb, r_nlh);
 
        return nlmsg_unicast(xfrm_net_nlsk(net, skb), r_skb, portid);