]> rtime.felk.cvut.cz Git - can-eth-gw.git/commitdiff
wrongly nested rcu_read_* fix
authorRadek Matejka <radek.matejka@gmail.com>
Fri, 17 Aug 2012 14:33:50 +0000 (16:33 +0200)
committerRadek Matejka <radek.matejka@gmail.com>
Fri, 17 Aug 2012 14:33:50 +0000 (16:33 +0200)
Rcu lock replaced with mutex.

kernel/canethgw.c
test/test

index 27f752516a41f7c8db854f5a58e78a2d05193ae6..1d1e95ec6f7e38b67f78a6a01158bfea4fa2ad17 100644 (file)
@@ -57,9 +57,11 @@ static struct socket *can_sock = NULL, *udp_sock = NULL;
 static struct task_struct *eth_to_can = NULL, *can_to_eth = NULL;
 static struct notifier_block notifier;
 
-HLIST_HEAD(rule_can_eth);
-HLIST_HEAD(rule_eth_can);
-DEFINE_MUTEX(cegw_mutex);
+static HLIST_HEAD(rule_eth_can);
+static HLIST_HEAD(rule_can_eth);
+static DEFINE_MUTEX(rule_eth_can_mutex);
+static DEFINE_MUTEX(rule_can_eth_mutex);
+static DEFINE_MUTEX(cegw_mutex);
 
 static void cegw_udp_send(struct socket *udp_sock, struct can_frame *cf,
                struct in_addr ipaddr, u16 port)
@@ -134,7 +136,7 @@ static int cegw_udp_can(void *data)
                        break;
                vec.iov_base = &cf;
                vec.iov_len = sizeof(cf);
-               recv_size = kernel_recvmsg(udp_sock, &mh, &vec, 1, 
+               recv_size = kernel_recvmsg(udp_sock, &mh, &vec, 1,
                                sizeof(cf), 0);
                /* recv_size == 0 when shutting down */
                if (recv_size != sizeof(cf) || recv_size == 0)
@@ -142,13 +144,13 @@ static int cegw_udp_can(void *data)
                else if (recv_size < 0)
                        return -1;
 
-               hlist_for_each_entry_rcu(rule, pos, &rule_eth_can, list) {
-                       rcu_read_lock();
+               mutex_lock(&rule_eth_can_mutex);
+               hlist_for_each_entry(rule, pos, &rule_eth_can, list) {
                        can_ifidx = rule->can_ifindex;
-                       rcu_read_unlock();
                        /* ToDo: from filter */
                        cegw_can_send(can_sock, &cf, can_ifidx);
                }
+               mutex_unlock(&rule_eth_can_mutex);
        }
 
        return 0;
@@ -188,14 +190,14 @@ static int cegw_can_udp(void* data)
                else if (recv_size < 0)
                        return -1;
 
-               hlist_for_each_entry_rcu(rule, pos, &rule_can_eth, list) {
-                       rcu_read_lock();
+               mutex_lock(&rule_can_eth_mutex);
+               hlist_for_each_entry(rule, pos, &rule_can_eth, list) {
                        eth_ip = rule->eth_ip;
                        eth_port = rule->eth_port;
-                       rcu_read_unlock();
                        if (rule->can_ifindex == ca.can_ifindex)
                                cegw_udp_send(udp_sock, &cf, eth_ip, eth_port);
                }
+               mutex_unlock(&rule_can_eth_mutex);
        }
 
        return 0;
@@ -246,19 +248,19 @@ static int cegw_newroute(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
                set->eth_port = *(unsigned short*)nla_data(tb[CEGW_ETH_PORT]);
                kthread_run(cegw_thread_start, set, "canethgw");
                break;
-       case CEGW_RULE_CAN_ETH:
+       case CEGW_RULE_ETH_CAN:
                if (!tb[CEGW_ETH_IP] || !tb[CEGW_ETH_PORT] || 
                    !tb[CEGW_CAN_IFINDEX]) {
-                       pr_devel("canethgw: missing attribute for "
-                                "CEGW_RULE_CAN_ETH\n");
+                       pr_devel("canethgw: missing attribute for"
+                                "CEGW_RULE_ETH_CAN\n");
                        return -EINVAL;
                }
 
                ifindex = *(int*)nla_data(tb[CEGW_CAN_IFINDEX]);
                ip = *(struct in_addr*)nla_data(tb[CEGW_ETH_IP]);
                port = *(unsigned short*)nla_data(tb[CEGW_ETH_PORT]);
-               pr_devel("canethgw: new can->eth rule - (%d)->(%x:%hu)\n",
-                        ifindex, ip.s_addr, port);
+               pr_devel("canethgw: new eth->can rule - (%x:%hu)->(%d)\n",
+                        ip.s_addr, port, ifindex);
 
                rule = kmalloc(sizeof(struct cegw_rule), GFP_KERNEL);
                if (rule == NULL)
@@ -268,21 +270,23 @@ static int cegw_newroute(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
                rule->eth_ip = ip;
                rule->eth_port = port;
 
-               hlist_add_head_rcu(&rule->list, &rule_can_eth);
+               mutex_lock(&rule_eth_can_mutex);
+               hlist_add_head(&rule->list, &rule_eth_can);
+               mutex_unlock(&rule_eth_can_mutex);
                break;
-       case CEGW_RULE_ETH_CAN:
+       case CEGW_RULE_CAN_ETH:
                if (!tb[CEGW_ETH_IP] || !tb[CEGW_ETH_PORT] || 
                    !tb[CEGW_CAN_IFINDEX]) {
-                       pr_devel("canethgw: missing attribute for"
-                                "CEGW_RULE_ETH_CAN\n");
+                       pr_devel("canethgw: missing attribute for "
+                                "CEGW_RULE_CAN_ETH\n");
                        return -EINVAL;
                }
 
                ifindex = *(int*)nla_data(tb[CEGW_CAN_IFINDEX]);
                ip = *(struct in_addr*)nla_data(tb[CEGW_ETH_IP]);
                port = *(unsigned short*)nla_data(tb[CEGW_ETH_PORT]);
-               pr_devel("canethgw: new eth->can rule - (%x:%hu)->(%d)\n",
-                        ip.s_addr, port, ifindex);
+               pr_devel("canethgw: new can->eth rule - (%d)->(%x:%hu)\n",
+                        ifindex, ip.s_addr, port);
 
                rule = kmalloc(sizeof(struct cegw_rule), GFP_KERNEL);
                if (rule == NULL)
@@ -292,7 +296,9 @@ static int cegw_newroute(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
                rule->eth_ip = ip;
                rule->eth_port = port;
 
-               hlist_add_head_rcu(&rule->list, &rule_eth_can);
+               mutex_lock(&rule_can_eth_mutex);
+               hlist_add_head(&rule->list, &rule_can_eth);
+               mutex_unlock(&rule_can_eth_mutex);
                break;
        default:
                pr_devel("canethgw: unknown CEGW_CMD_INFO\n");
@@ -307,14 +313,19 @@ static void cegw_flush(void)
        struct cegw_rule *rule;
        struct hlist_node *pos, *n;
 
-       hlist_for_each_entry_safe(rule, pos, n, &rule_can_eth, list) {
+       mutex_lock(&rule_eth_can_mutex);
+       hlist_for_each_entry_safe(rule, pos, n, &rule_eth_can, list) {
                hlist_del(&rule->list);
                kfree(rule);
        }
-       hlist_for_each_entry_safe(rule, pos, n, &rule_eth_can, list) {
+       mutex_unlock(&rule_eth_can_mutex);
+
+       mutex_lock(&rule_can_eth_mutex);
+       hlist_for_each_entry_safe(rule, pos, n, &rule_can_eth, list) {
                hlist_del(&rule->list);
                kfree(rule);
        }
+       mutex_unlock(&rule_can_eth_mutex);
 }
 
 static int cegw_delroute(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
@@ -402,8 +413,9 @@ static int cegw_getroute(struct sk_buff *skb, struct netlink_callback *cb)
        int idx = 0;
        int s_idx = cb->args[0];
 
-       rcu_read_lock();
-       hlist_for_each_entry_rcu(rule, pos, &rule_eth_can, list) {
+       mutex_lock(&rule_eth_can_mutex);
+       mutex_lock(&rule_can_eth_mutex);
+       hlist_for_each_entry(rule, pos, &rule_eth_can, list) {
                if (idx < s_idx)
                        goto cont1;
 
@@ -413,7 +425,7 @@ cont1:
                idx++;
        }
 
-       hlist_for_each_entry_rcu(rule, pos, &rule_can_eth, list) {
+       hlist_for_each_entry(rule, pos, &rule_can_eth, list) {
                if (idx < s_idx)
                        goto cont2;
 
@@ -424,7 +436,8 @@ cont2:
        }
 
 brk:
-       rcu_read_unlock();
+       mutex_unlock(&rule_eth_can_mutex);
+       mutex_unlock(&rule_can_eth_mutex);
        cb->args[0] = idx;
 
        return skb->len;
index 44b0ecd756164da4eaa9416be0ebb9d42d769677..419b1e502085e3c3322f62b1f78468f6a40a2a16 100755 (executable)
--- a/test/test
+++ b/test/test
@@ -1,4 +1,6 @@
 #!/bin/sh
+N=10
+
 evalmsg()
 {
        if test $1 -eq 0; then
@@ -9,11 +11,15 @@ evalmsg()
                echo -ne "\033[0m"
 }
 
+if test $# -ne 0; then
+       N=$1;
+fi
+
 echo -n "Testing eth->can: "
-cegwbench -s udp@127.0.0.1:10501 -d can@vcan0 -n 10 -t 1 &> /dev/zero
+cegwbench -s udp@127.0.0.1:10501 -d can@vcan0 -n $N -t 1 &> /dev/zero
 evalmsg $?
 
 echo -n "Testing can->eth: "
-cegwbench -s can@vcan0 -d udp@127.0.0.1:10502 -n 10 -t 1 &> /dev/zero
+cegwbench -s can@vcan0 -d udp@127.0.0.1:10502 -n $N -t 1 &> /dev/zero
 evalmsg $?