[NETFILTER]: ip_conntrack: properly use RCU API for ip_ct_protos array
Patrick McHardy [Mon, 12 Feb 2007 19:12:40 +0000 (11:12 -0800)]
Replace preempt_{enable,disable} based RCU by proper use of the
RCU API and add missing rcu_read_lock/rcu_read_unlock calls in
all paths not obviously only used within packet process context
(nfnetlink_conntrack).

Signed-off-by: Patrick McHardy <kaber@trash.net>
Signed-off-by: David S. Miller <davem@davemloft.net>

net/ipv4/netfilter/ip_conntrack_core.c
net/ipv4/netfilter/ip_conntrack_standalone.c
net/ipv4/netfilter/ip_nat_core.c

index 508e600..e7de6d3 100644 (file)
@@ -318,9 +318,11 @@ destroy_conntrack(struct nf_conntrack *nfct)
        /* To make sure we don't get any weird locking issues here:
         * destroy_conntrack() MUST NOT be called with a write lock
         * to ip_conntrack_lock!!! -HW */
+       rcu_read_lock();
        proto = __ip_conntrack_proto_find(ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.protonum);
        if (proto && proto->destroy)
                proto->destroy(ct);
+       rcu_read_unlock();
 
        if (ip_conntrack_destroyed)
                ip_conntrack_destroyed(ct);
@@ -595,13 +597,13 @@ ip_conntrack_proto_find_get(u_int8_t protocol)
 {
        struct ip_conntrack_protocol *p;
 
-       preempt_disable();
+       rcu_read_lock();
        p = __ip_conntrack_proto_find(protocol);
        if (p) {
                if (!try_module_get(p->me))
                        p = &ip_conntrack_generic_protocol;
        }
-       preempt_enable();
+       rcu_read_unlock();
 
        return p;
 }
@@ -830,6 +832,7 @@ unsigned int ip_conntrack_in(unsigned int hooknum,
        }
 #endif
 
+       /* rcu_read_lock()ed by nf_hook_slow */
        proto = __ip_conntrack_proto_find((*pskb)->nh.iph->protocol);
 
        /* It may be an special packet, error, unclean...
@@ -875,8 +878,15 @@ unsigned int ip_conntrack_in(unsigned int hooknum,
 int invert_tuplepr(struct ip_conntrack_tuple *inverse,
                   const struct ip_conntrack_tuple *orig)
 {
-       return ip_ct_invert_tuple(inverse, orig,
-                                 __ip_conntrack_proto_find(orig->dst.protonum));
+       struct ip_conntrack_protocol *proto;
+       int ret;
+
+       rcu_read_lock();
+       proto = __ip_conntrack_proto_find(orig->dst.protonum);
+       ret = ip_ct_invert_tuple(inverse, orig, proto);
+       rcu_read_unlock();
+
+       return ret;
 }
 
 /* Would two expected things clash? */
@@ -1507,11 +1517,11 @@ int __init ip_conntrack_init(void)
        /* Don't NEED lock here, but good form anyway. */
        write_lock_bh(&ip_conntrack_lock);
        for (i = 0; i < MAX_IP_CT_PROTO; i++)
-               ip_ct_protos[i] = &ip_conntrack_generic_protocol;
+               rcu_assign_pointer(ip_ct_protos[i], &ip_conntrack_generic_protocol);
        /* Sew in builtin protocols. */
-       ip_ct_protos[IPPROTO_TCP] = &ip_conntrack_protocol_tcp;
-       ip_ct_protos[IPPROTO_UDP] = &ip_conntrack_protocol_udp;
-       ip_ct_protos[IPPROTO_ICMP] = &ip_conntrack_protocol_icmp;
+       rcu_assign_pointer(ip_ct_protos[IPPROTO_TCP], &ip_conntrack_protocol_tcp);
+       rcu_assign_pointer(ip_ct_protos[IPPROTO_UDP], &ip_conntrack_protocol_udp);
+       rcu_assign_pointer(ip_ct_protos[IPPROTO_ICMP], &ip_conntrack_protocol_icmp);
        write_unlock_bh(&ip_conntrack_lock);
 
        /* For use by ipt_REJECT */
index 300ccbb..c7c1ec6 100644 (file)
@@ -796,7 +796,7 @@ int ip_conntrack_protocol_register(struct ip_conntrack_protocol *proto)
                ret = -EBUSY;
                goto out;
        }
-       ip_ct_protos[proto->proto] = proto;
+       rcu_assign_pointer(ip_ct_protos[proto->proto], proto);
  out:
        write_unlock_bh(&ip_conntrack_lock);
        return ret;
@@ -805,11 +805,10 @@ int ip_conntrack_protocol_register(struct ip_conntrack_protocol *proto)
 void ip_conntrack_protocol_unregister(struct ip_conntrack_protocol *proto)
 {
        write_lock_bh(&ip_conntrack_lock);
-       ip_ct_protos[proto->proto] = &ip_conntrack_generic_protocol;
+       rcu_assign_pointer(ip_ct_protos[proto->proto],
+                          &ip_conntrack_generic_protocol);
        write_unlock_bh(&ip_conntrack_lock);
-
-       /* Somebody could be still looking at the proto in bh. */
-       synchronize_net();
+       synchronize_rcu();
 
        /* Remove all contrack entries for this protocol */
        ip_ct_iterate_cleanup(kill_proto, &proto->proto);
index 85ae0ca..18daabc 100644 (file)
@@ -420,6 +420,7 @@ int ip_nat_icmp_reply_translation(struct ip_conntrack *ct,
                struct icmphdr icmp;
                struct iphdr ip;
        } *inside;
+       struct ip_conntrack_protocol *proto;
        struct ip_conntrack_tuple inner, target;
        int hdrlen = (*pskb)->nh.iph->ihl * 4;
        enum ip_conntrack_dir dir = CTINFO2DIR(ctinfo);
@@ -455,10 +456,11 @@ int ip_nat_icmp_reply_translation(struct ip_conntrack *ct,
        DEBUGP("icmp_reply_translation: translating error %p manp %u dir %s\n",
               *pskb, manip, dir == IP_CT_DIR_ORIGINAL ? "ORIG" : "REPLY");
 
+       /* rcu_read_lock()ed by nf_hook_slow */
+       proto = __ip_conntrack_proto_find(inside->ip.protocol);
        if (!ip_ct_get_tuple(&inside->ip, *pskb, (*pskb)->nh.iph->ihl*4 +
                             sizeof(struct icmphdr) + inside->ip.ihl*4,
-                            &inner,
-                            __ip_conntrack_proto_find(inside->ip.protocol)))
+                            &inner, proto))
                return 0;
 
        /* Change inner back to look like incoming packet.  We do the