Author: simon Date: 2010-01-01 21:09:34 +0100 (Fri, 01 Jan 2010) New Revision: 1529
Modified: trunk/batman-adv-kernelland/hard-interface.c trunk/batman-adv-kernelland/routing.c Log: batman-adv: fix unlock, make code sparse clean
If the skb failed to copy in the (batman) icmp receive handlers, the function returned without unlocking the orig_hash_lock first. To prevent this, the fields needed from the orig_node are now copied, and the copies are used for sending (just like in other handlers).
Furthermore, find_batman_if() was declared static. The code now appears to be clean in 'sparse' [1].
Thanks to Andrew Lunn for pointing that out.
[1] http://sparse.wiki.kernel.org/index.php/Main_Page
Signed-off-by: Simon Wunderlich siwu@hrz.tu-chemnitz.de
Modified: trunk/batman-adv-kernelland/hard-interface.c =================================================================== --- trunk/batman-adv-kernelland/hard-interface.c 2010-01-01 17:13:27 UTC (rev 1528) +++ trunk/batman-adv-kernelland/hard-interface.c 2010-01-01 20:09:34 UTC (rev 1529) @@ -401,7 +401,7 @@ }
/* find batman interface by netdev. assumes rcu_read_lock on */ -struct batman_if *find_batman_if(struct net_device *dev) +static struct batman_if *find_batman_if(struct net_device *dev) { struct batman_if *batman_if;
Modified: trunk/batman-adv-kernelland/routing.c =================================================================== --- trunk/batman-adv-kernelland/routing.c 2010-01-01 17:13:27 UTC (rev 1528) +++ trunk/batman-adv-kernelland/routing.c 2010-01-01 20:09:34 UTC (rev 1529) @@ -589,11 +589,13 @@ struct icmp_packet *icmp_packet; struct ethhdr *ethhdr; struct sk_buff *skb_old; + struct batman_if *batman_if; int ret; unsigned long flags; + uint8_t dstaddr[ETH_ALEN];
icmp_packet = (struct icmp_packet *) skb->data; - ethhdr = (struct ethhdr *)skb_mac_header(skb); + ethhdr = (struct ethhdr *) skb_mac_header(skb);
/* add data to device queue */ if (icmp_packet->msg_type != ECHO_REQUEST) { @@ -612,6 +614,12 @@ (orig_node->batman_if != NULL) && (orig_node->router != NULL)) {
+ /* don't lock while sending the packets ... we therefore + * copy the required data before sending */ + batman_if = orig_node->batman_if; + memcpy(dstaddr, orig_node->router->addr, ETH_ALEN); + spin_unlock_irqrestore(&orig_hash_lock, flags); + /* create a copy of the skb, if needed, to modify it. */ skb_old = NULL; if (!skb_clone_writable(skb, sizeof(struct icmp_packet))) { @@ -628,13 +636,12 @@ icmp_packet->msg_type = ECHO_REPLY; icmp_packet->ttl = TTL;
- send_skb_packet(skb, - orig_node->batman_if, - orig_node->router->addr); + send_skb_packet(skb, batman_if, dstaddr); ret = NET_RX_SUCCESS; - }
- spin_unlock_irqrestore(&orig_hash_lock, flags); + } else + spin_unlock_irqrestore(&orig_hash_lock, flags); + return ret; }
@@ -644,8 +651,10 @@ struct icmp_packet *icmp_packet; struct ethhdr *ethhdr; struct sk_buff *skb_old; + struct batman_if *batman_if; int ret; unsigned long flags; + uint8_t dstaddr[ETH_ALEN];
icmp_packet = (struct icmp_packet *) skb->data; ethhdr = (struct ethhdr *) skb_mac_header(skb); @@ -666,6 +675,12 @@ (orig_node->batman_if != NULL) && (orig_node->router != NULL)) {
+ /* don't lock while sending the packets ... we therefore + * copy the required data before sending */ + batman_if = orig_node->batman_if; + memcpy(dstaddr, orig_node->router->addr, ETH_ALEN); + spin_unlock_irqrestore(&orig_hash_lock, flags); + /* create a copy of the skb, if needed, to modify it. */ if (!skb_clone_writable(skb, sizeof(struct icmp_packet))) { skb_old = skb; @@ -681,14 +696,12 @@ icmp_packet->msg_type = TTL_EXCEEDED; icmp_packet->ttl = TTL;
- send_skb_packet(skb, - orig_node->batman_if, - orig_node->router->addr); + send_skb_packet(skb, batman_if, dstaddr); ret = NET_RX_SUCCESS;
- } + } else + spin_unlock_irqrestore(&orig_hash_lock, flags);
- spin_unlock_irqrestore(&orig_hash_lock, flags); return ret; }
@@ -699,9 +712,11 @@ struct ethhdr *ethhdr; struct orig_node *orig_node; struct sk_buff *skb_old; + struct batman_if *batman_if; int hdr_size = sizeof(struct icmp_packet); int ret; unsigned long flags; + uint8_t dstaddr[ETH_ALEN];
/* drop packet if it has not necessary minimum size */ if (skb_headlen(skb) < hdr_size) @@ -744,6 +759,12 @@ (orig_node->batman_if != NULL) && (orig_node->router != NULL)) {
+ /* don't lock while sending the packets ... we therefore + * copy the required data before sending */ + batman_if = orig_node->batman_if; + memcpy(dstaddr, orig_node->router->addr, ETH_ALEN); + spin_unlock_irqrestore(&orig_hash_lock, flags); + /* create a copy of the skb, if needed, to modify it. */ if (!skb_clone_writable(skb, sizeof(struct icmp_packet))) { skb_old = skb; @@ -758,12 +779,12 @@ icmp_packet->ttl--;
/* route it */ - send_skb_packet(skb, - orig_node->batman_if, - orig_node->router->addr); + send_skb_packet(skb, batman_if, dstaddr); ret = NET_RX_SUCCESS; - } - spin_unlock_irqrestore(&orig_hash_lock, flags); + + } else + spin_unlock_irqrestore(&orig_hash_lock, flags); + return ret; }
@@ -842,8 +863,10 @@ /* route it */ send_skb_packet(skb, batman_if, dstaddr); ret = NET_RX_SUCCESS; + } else spin_unlock_irqrestore(&orig_hash_lock, flags); + return ret; }