ipv6/addrconf: annotate data-races around devconf fields (II)
authorEric Dumazet <edumazet@google.com>
Wed, 28 Feb 2024 13:54:38 +0000 (13:54 +0000)
committerDavid S. Miller <davem@davemloft.net>
Fri, 1 Mar 2024 08:42:33 +0000 (08:42 +0000)
Final (?) round of this series.

Annotate lockless reads on following devconf fields,
because they be changed concurrently from /proc/net/ipv6/conf.

- accept_dad
- optimistic_dad
- use_optimistic
- use_oif_addrs_only
- ra_honor_pio_life
- keep_addr_on_down
- ndisc_notify
- ndisc_evict_nocarrier
- suppress_frag_ndisc
- addr_gen_mode
- seg6_enabled
- ioam6_enabled
- ioam6_id
- ioam6_id_wide
- drop_unicast_in_l2_multicast
- mldv[12]_unsolicited_report_interval
- force_mld_version
- force_tllao
- accept_untracked_na
- drop_unsolicited_na
- accept_source_route

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/ipv6/addrconf.c
net/ipv6/exthdrs.c
net/ipv6/ioam6.c
net/ipv6/ip6_input.c
net/ipv6/mcast.c
net/ipv6/ndisc.c
net/ipv6/seg6_hmac.c

index bcfe725f8fbd43220d4971d4c30623964aedb30f..436d9a6e6a82e1a0b99e190aa54255460706c565 100644 (file)
@@ -1557,15 +1557,17 @@ static inline int ipv6_saddr_preferred(int type)
        return 0;
 }
 
-static bool ipv6_use_optimistic_addr(struct net *net,
-                                    struct inet6_dev *idev)
+static bool ipv6_use_optimistic_addr(const struct net *net,
+                                    const struct inet6_dev *idev)
 {
 #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
        if (!idev)
                return false;
-       if (!net->ipv6.devconf_all->optimistic_dad && !idev->cnf.optimistic_dad)
+       if (!READ_ONCE(net->ipv6.devconf_all->optimistic_dad) &&
+           !READ_ONCE(idev->cnf.optimistic_dad))
                return false;
-       if (!net->ipv6.devconf_all->use_optimistic && !idev->cnf.use_optimistic)
+       if (!READ_ONCE(net->ipv6.devconf_all->use_optimistic) &&
+           !READ_ONCE(idev->cnf.use_optimistic))
                return false;
 
        return true;
@@ -1574,13 +1576,14 @@ static bool ipv6_use_optimistic_addr(struct net *net,
 #endif
 }
 
-static bool ipv6_allow_optimistic_dad(struct net *net,
-                                     struct inet6_dev *idev)
+static bool ipv6_allow_optimistic_dad(const struct net *net,
+                                     const struct inet6_dev *idev)
 {
 #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
        if (!idev)
                return false;
-       if (!net->ipv6.devconf_all->optimistic_dad && !idev->cnf.optimistic_dad)
+       if (!READ_ONCE(net->ipv6.devconf_all->optimistic_dad) &&
+           !READ_ONCE(idev->cnf.optimistic_dad))
                return false;
 
        return true;
@@ -1862,7 +1865,7 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dst_dev,
                idev = __in6_dev_get(dst_dev);
                if ((dst_type & IPV6_ADDR_MULTICAST) ||
                    dst.scope <= IPV6_ADDR_SCOPE_LINKLOCAL ||
-                   (idev && idev->cnf.use_oif_addrs_only)) {
+                   (idev && READ_ONCE(idev->cnf.use_oif_addrs_only))) {
                        use_oif_addr = true;
                }
        }
@@ -2683,8 +2686,8 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
                };
 
 #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
-               if ((net->ipv6.devconf_all->optimistic_dad ||
-                    in6_dev->cnf.optimistic_dad) &&
+               if ((READ_ONCE(net->ipv6.devconf_all->optimistic_dad) ||
+                    READ_ONCE(in6_dev->cnf.optimistic_dad)) &&
                    !net->ipv6.devconf_all->forwarding && sllao)
                        cfg.ifa_flags |= IFA_F_OPTIMISTIC;
 #endif
@@ -2733,7 +2736,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
                 */
                update_lft = !create && stored_lft;
 
-               if (update_lft && !in6_dev->cnf.ra_honor_pio_life) {
+               if (update_lft && !READ_ONCE(in6_dev->cnf.ra_honor_pio_life)) {
                        const u32 minimum_lft = min_t(u32,
                                stored_lft, MIN_VALID_LIFETIME);
                        valid_lft = max(valid_lft, minimum_lft);
@@ -3317,8 +3320,8 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
        struct inet6_ifaddr *ifp;
 
 #ifdef CONFIG_IPV6_OPTIMISTIC_DAD
-       if ((dev_net(idev->dev)->ipv6.devconf_all->optimistic_dad ||
-            idev->cnf.optimistic_dad) &&
+       if ((READ_ONCE(dev_net(idev->dev)->ipv6.devconf_all->optimistic_dad) ||
+            READ_ONCE(idev->cnf.optimistic_dad)) &&
            !dev_net(idev->dev)->ipv6.devconf_all->forwarding)
                cfg.ifa_flags |= IFA_F_OPTIMISTIC;
 #endif
@@ -3890,10 +3893,10 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister)
         */
        if (!unregister && !idev->cnf.disable_ipv6) {
                /* aggregate the system setting and interface setting */
-               int _keep_addr = net->ipv6.devconf_all->keep_addr_on_down;
+               int _keep_addr = READ_ONCE(net->ipv6.devconf_all->keep_addr_on_down);
 
                if (!_keep_addr)
-                       _keep_addr = idev->cnf.keep_addr_on_down;
+                       _keep_addr = READ_ONCE(idev->cnf.keep_addr_on_down);
 
                keep_addr = (_keep_addr > 0);
        }
@@ -4119,8 +4122,8 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp)
 
        net = dev_net(dev);
        if (dev->flags&(IFF_NOARP|IFF_LOOPBACK) ||
-           (net->ipv6.devconf_all->accept_dad < 1 &&
-            idev->cnf.accept_dad < 1) ||
+           (READ_ONCE(net->ipv6.devconf_all->accept_dad) < 1 &&
+            READ_ONCE(idev->cnf.accept_dad) < 1) ||
            !(ifp->flags&IFA_F_TENTATIVE) ||
            ifp->flags & IFA_F_NODAD) {
                bool send_na = false;
@@ -4212,8 +4215,8 @@ static void addrconf_dad_work(struct work_struct *w)
                action = DAD_ABORT;
                ifp->state = INET6_IFADDR_STATE_POSTDAD;
 
-               if ((dev_net(idev->dev)->ipv6.devconf_all->accept_dad > 1 ||
-                    idev->cnf.accept_dad > 1) &&
+               if ((READ_ONCE(dev_net(idev->dev)->ipv6.devconf_all->accept_dad) > 1 ||
+                    READ_ONCE(idev->cnf.accept_dad) > 1) &&
                    !idev->cnf.disable_ipv6 &&
                    !(ifp->flags & IFA_F_STABLE_PRIVACY)) {
                        struct in6_addr addr;
@@ -4352,8 +4355,8 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp, bool bump_id,
 
        /* send unsolicited NA if enabled */
        if (send_na &&
-           (ifp->idev->cnf.ndisc_notify ||
-            dev_net(dev)->ipv6.devconf_all->ndisc_notify)) {
+           (READ_ONCE(ifp->idev->cnf.ndisc_notify) ||
+            READ_ONCE(dev_net(dev)->ipv6.devconf_all->ndisc_notify))) {
                ndisc_send_na(dev, &in6addr_linklocal_allnodes, &ifp->addr,
                              /*router=*/ !!ifp->idev->cnf.forwarding,
                              /*solicited=*/ false, /*override=*/ true,
@@ -6541,7 +6544,7 @@ static int addrconf_sysctl_addr_gen_mode(struct ctl_table *ctl, int write,
                } else if (&net->ipv6.devconf_all->addr_gen_mode == ctl->data) {
                        struct net_device *dev;
 
-                       net->ipv6.devconf_dflt->addr_gen_mode = new_val;
+                       WRITE_ONCE(net->ipv6.devconf_dflt->addr_gen_mode, new_val);
                        for_each_netdev(net, dev) {
                                idev = __in6_dev_get(dev);
                                if (idev &&
@@ -6553,7 +6556,7 @@ static int addrconf_sysctl_addr_gen_mode(struct ctl_table *ctl, int write,
                        }
                }
 
-               *((u32 *)ctl->data) = new_val;
+               WRITE_ONCE(*((u32 *)ctl->data), new_val);
        }
 
 out:
index 727792907d6cb2132d965da4c7cb8447f07b2a68..26f1e4a5ade05175375add7da8da1fde9b204d96 100644 (file)
@@ -379,9 +379,8 @@ static int ipv6_srh_rcv(struct sk_buff *skb)
 
        idev = __in6_dev_get(skb->dev);
 
-       accept_seg6 = net->ipv6.devconf_all->seg6_enabled;
-       if (accept_seg6 > idev->cnf.seg6_enabled)
-               accept_seg6 = idev->cnf.seg6_enabled;
+       accept_seg6 = min(READ_ONCE(net->ipv6.devconf_all->seg6_enabled),
+                         READ_ONCE(idev->cnf.seg6_enabled));
 
        if (!accept_seg6) {
                kfree_skb(skb);
@@ -655,10 +654,13 @@ static int ipv6_rthdr_rcv(struct sk_buff *skb)
        struct ipv6_rt_hdr *hdr;
        struct rt0_hdr *rthdr;
        struct net *net = dev_net(skb->dev);
-       int accept_source_route = net->ipv6.devconf_all->accept_source_route;
+       int accept_source_route;
 
-       if (idev && accept_source_route > idev->cnf.accept_source_route)
-               accept_source_route = idev->cnf.accept_source_route;
+       accept_source_route = READ_ONCE(net->ipv6.devconf_all->accept_source_route);
+
+       if (idev)
+               accept_source_route = min(accept_source_route,
+                                         READ_ONCE(idev->cnf.accept_source_route));
 
        if (!pskb_may_pull(skb, skb_transport_offset(skb) + 8) ||
            !pskb_may_pull(skb, (skb_transport_offset(skb) +
@@ -919,7 +921,7 @@ static bool ipv6_hop_ioam(struct sk_buff *skb, int optoff)
                goto drop;
 
        /* Ignore if IOAM is not enabled on ingress */
-       if (!__in6_dev_get(skb->dev)->cnf.ioam6_enabled)
+       if (!READ_ONCE(__in6_dev_get(skb->dev)->cnf.ioam6_enabled))
                goto ignore;
 
        /* Truncated Option header */
index 5fa923f0663214df4783344e0f790b368a2bf1bf..08c929513065772dca3666c92326a47f2dae0631 100644 (file)
@@ -727,7 +727,7 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
                if (!skb->dev)
                        raw16 = IOAM6_U16_UNAVAILABLE;
                else
-                       raw16 = (__force u16)__in6_dev_get(skb->dev)->cnf.ioam6_id;
+                       raw16 = (__force u16)READ_ONCE(__in6_dev_get(skb->dev)->cnf.ioam6_id);
 
                *(__be16 *)data = cpu_to_be16(raw16);
                data += sizeof(__be16);
@@ -735,7 +735,7 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
                if (skb_dst(skb)->dev->flags & IFF_LOOPBACK)
                        raw16 = IOAM6_U16_UNAVAILABLE;
                else
-                       raw16 = (__force u16)__in6_dev_get(skb_dst(skb)->dev)->cnf.ioam6_id;
+                       raw16 = (__force u16)READ_ONCE(__in6_dev_get(skb_dst(skb)->dev)->cnf.ioam6_id);
 
                *(__be16 *)data = cpu_to_be16(raw16);
                data += sizeof(__be16);
@@ -822,7 +822,7 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
                if (!skb->dev)
                        raw32 = IOAM6_U32_UNAVAILABLE;
                else
-                       raw32 = __in6_dev_get(skb->dev)->cnf.ioam6_id_wide;
+                       raw32 = READ_ONCE(__in6_dev_get(skb->dev)->cnf.ioam6_id_wide);
 
                *(__be32 *)data = cpu_to_be32(raw32);
                data += sizeof(__be32);
@@ -830,7 +830,7 @@ static void __ioam6_fill_trace_data(struct sk_buff *skb,
                if (skb_dst(skb)->dev->flags & IFF_LOOPBACK)
                        raw32 = IOAM6_U32_UNAVAILABLE;
                else
-                       raw32 = __in6_dev_get(skb_dst(skb)->dev)->cnf.ioam6_id_wide;
+                       raw32 = READ_ONCE(__in6_dev_get(skb_dst(skb)->dev)->cnf.ioam6_id_wide);
 
                *(__be32 *)data = cpu_to_be32(raw32);
                data += sizeof(__be32);
index 1ba97933c74fbd12e21f273f0aeda2313bd608b7..133610a49da6b8a2a210ad8faf74015c6bdf7038 100644 (file)
@@ -236,7 +236,7 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
        if (!ipv6_addr_is_multicast(&hdr->daddr) &&
            (skb->pkt_type == PACKET_BROADCAST ||
             skb->pkt_type == PACKET_MULTICAST) &&
-           idev->cnf.drop_unicast_in_l2_multicast) {
+           READ_ONCE(idev->cnf.drop_unicast_in_l2_multicast)) {
                SKB_DR_SET(reason, UNICAST_IN_L2_MULTICAST);
                goto err;
        }
index 76ee1615ff2a07e1dd496aada377a7feb4703e8c..7ba01d8cfbae839d87e0c85729f7bed9ba328f05 100644 (file)
@@ -159,9 +159,9 @@ static int unsolicited_report_interval(struct inet6_dev *idev)
        int iv;
 
        if (mld_in_v1_mode(idev))
-               iv = idev->cnf.mldv1_unsolicited_report_interval;
+               iv = READ_ONCE(idev->cnf.mldv1_unsolicited_report_interval);
        else
-               iv = idev->cnf.mldv2_unsolicited_report_interval;
+               iv = READ_ONCE(idev->cnf.mldv2_unsolicited_report_interval);
 
        return iv > 0 ? iv : 1;
 }
@@ -1202,15 +1202,15 @@ static bool mld_marksources(struct ifmcaddr6 *pmc, int nsrcs,
 
 static int mld_force_mld_version(const struct inet6_dev *idev)
 {
+       const struct net *net = dev_net(idev->dev);
+       int all_force;
+
+       all_force = READ_ONCE(net->ipv6.devconf_all->force_mld_version);
        /* Normally, both are 0 here. If enforcement to a particular is
         * being used, individual device enforcement will have a lower
         * precedence over 'all' device (.../conf/all/force_mld_version).
         */
-
-       if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
-               return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
-       else
-               return idev->cnf.force_mld_version;
+       return all_force ?: READ_ONCE(idev->cnf.force_mld_version);
 }
 
 static bool mld_in_v2_mode_only(const struct inet6_dev *idev)
index 4114918f12c88f2b74e53d6d726018994feaf213..ae134634c323cab27c03328015b24ae397f97cfc 100644 (file)
@@ -451,7 +451,7 @@ static void ip6_nd_hdr(struct sk_buff *skb,
 
        rcu_read_lock();
        idev = __in6_dev_get(skb->dev);
-       tclass = idev ? idev->cnf.ndisc_tclass : 0;
+       tclass = idev ? READ_ONCE(idev->cnf.ndisc_tclass) : 0;
        rcu_read_unlock();
 
        skb_push(skb, sizeof(*hdr));
@@ -535,7 +535,7 @@ void ndisc_send_na(struct net_device *dev, const struct in6_addr *daddr,
                src_addr = solicited_addr;
                if (ifp->flags & IFA_F_OPTIMISTIC)
                        override = false;
-               inc_opt |= ifp->idev->cnf.force_tllao;
+               inc_opt |= READ_ONCE(ifp->idev->cnf.force_tllao);
                in6_ifa_put(ifp);
        } else {
                if (ipv6_dev_get_saddr(dev_net(dev), dev, daddr,
@@ -974,7 +974,7 @@ static int accept_untracked_na(struct net_device *dev, struct in6_addr *saddr)
 {
        struct inet6_dev *idev = __in6_dev_get(dev);
 
-       switch (idev->cnf.accept_untracked_na) {
+       switch (READ_ONCE(idev->cnf.accept_untracked_na)) {
        case 0: /* Don't accept untracked na (absent in neighbor cache) */
                return 0;
        case 1: /* Create new entries from na if currently untracked */
@@ -1025,7 +1025,7 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)
         * drop_unsolicited_na takes precedence over accept_untracked_na
         */
        if (!msg->icmph.icmp6_solicited && idev &&
-           idev->cnf.drop_unsolicited_na)
+           READ_ONCE(idev->cnf.drop_unsolicited_na))
                return reason;
 
        if (!ndisc_parse_options(dev, msg->opt, ndoptlen, &ndopts))
@@ -1818,7 +1818,7 @@ static bool ndisc_suppress_frag_ndisc(struct sk_buff *skb)
        if (!idev)
                return true;
        if (IP6CB(skb)->flags & IP6SKB_FRAGMENTED &&
-           idev->cnf.suppress_frag_ndisc) {
+           READ_ONCE(idev->cnf.suppress_frag_ndisc)) {
                net_warn_ratelimited("Received fragmented ndisc packet. Carefully consider disabling suppress_frag_ndisc.\n");
                return true;
        }
@@ -1895,8 +1895,8 @@ static int ndisc_netdev_event(struct notifier_block *this, unsigned long event,
                idev = in6_dev_get(dev);
                if (!idev)
                        break;
-               if (idev->cnf.ndisc_notify ||
-                   net->ipv6.devconf_all->ndisc_notify)
+               if (READ_ONCE(idev->cnf.ndisc_notify) ||
+                   READ_ONCE(net->ipv6.devconf_all->ndisc_notify))
                        ndisc_send_unsol_na(dev);
                in6_dev_put(idev);
                break;
@@ -1905,8 +1905,8 @@ static int ndisc_netdev_event(struct notifier_block *this, unsigned long event,
                if (!idev)
                        evict_nocarrier = true;
                else {
-                       evict_nocarrier = idev->cnf.ndisc_evict_nocarrier &&
-                                         net->ipv6.devconf_all->ndisc_evict_nocarrier;
+                       evict_nocarrier = READ_ONCE(idev->cnf.ndisc_evict_nocarrier) &&
+                                         READ_ONCE(net->ipv6.devconf_all->ndisc_evict_nocarrier);
                        in6_dev_put(idev);
                }
 
index d43c50a7310d64e3af88657286a431057e9577bd..861e0366f549d523f20dc92c79bef1be8805e0c7 100644 (file)
@@ -241,6 +241,7 @@ bool seg6_hmac_validate_skb(struct sk_buff *skb)
        struct sr6_tlv_hmac *tlv;
        struct ipv6_sr_hdr *srh;
        struct inet6_dev *idev;
+       int require_hmac;
 
        idev = __in6_dev_get(skb->dev);
 
@@ -248,16 +249,17 @@ bool seg6_hmac_validate_skb(struct sk_buff *skb)
 
        tlv = seg6_get_tlv_hmac(srh);
 
+       require_hmac = READ_ONCE(idev->cnf.seg6_require_hmac);
        /* mandatory check but no tlv */
-       if (idev->cnf.seg6_require_hmac > 0 && !tlv)
+       if (require_hmac > 0 && !tlv)
                return false;
 
        /* no check */
-       if (idev->cnf.seg6_require_hmac < 0)
+       if (require_hmac < 0)
                return true;
 
        /* check only if present */
-       if (idev->cnf.seg6_require_hmac == 0 && !tlv)
+       if (require_hmac == 0 && !tlv)
                return true;
 
        /* now, seg6_require_hmac >= 0 && tlv */