forked from Minki/linux
[NDISC]: Fix race in generic address resolution
Frank Blaschka provided the bug report and the initial suggested fix for this bug. He also validated this version of this fix. The problem is that the access to neigh->arp_queue is inconsistent, we grab references when dropping the lock lock to call neigh->ops->solicit() but this does not prevent other threads of control from trying to send out that packet at the same time causing corruptions because both code paths believe they have exclusive access to the skb. The best option seems to be to hold the write lock on neigh->lock during the ->solicit() call. I looked at all of the ndisc_ops implementations and this seems workable. The only case that needs special care is the IPV4 ARP implementation of arp_solicit(). It wants to take neigh->lock as a reader to protect the header entry in neigh->ha during the emission of the soliciation. We can simply remove the read lock calls to take care of that since holding the lock as a writer at the caller providers a superset of the protection afforded by the existing read locking. The rest of the ->solicit() implementations don't care whether the neigh is locked or not. Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
3611f4d2a5
commit
69cc64d8d9
@ -834,18 +834,12 @@ static void neigh_timer_handler(unsigned long arg)
|
||||
}
|
||||
if (neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) {
|
||||
struct sk_buff *skb = skb_peek(&neigh->arp_queue);
|
||||
/* keep skb alive even if arp_queue overflows */
|
||||
if (skb)
|
||||
skb_get(skb);
|
||||
write_unlock(&neigh->lock);
|
||||
|
||||
neigh->ops->solicit(neigh, skb);
|
||||
atomic_inc(&neigh->probes);
|
||||
if (skb)
|
||||
kfree_skb(skb);
|
||||
} else {
|
||||
out:
|
||||
write_unlock(&neigh->lock);
|
||||
}
|
||||
out:
|
||||
write_unlock(&neigh->lock);
|
||||
|
||||
if (notify)
|
||||
neigh_update_notify(neigh);
|
||||
|
@ -368,7 +368,6 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
|
||||
if (!(neigh->nud_state&NUD_VALID))
|
||||
printk(KERN_DEBUG "trying to ucast probe in NUD_INVALID\n");
|
||||
dst_ha = neigh->ha;
|
||||
read_lock_bh(&neigh->lock);
|
||||
} else if ((probes -= neigh->parms->app_probes) < 0) {
|
||||
#ifdef CONFIG_ARPD
|
||||
neigh_app_ns(neigh);
|
||||
@ -378,8 +377,6 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb)
|
||||
|
||||
arp_send(ARPOP_REQUEST, ETH_P_ARP, target, dev, saddr,
|
||||
dst_ha, dev->dev_addr, NULL);
|
||||
if (dst_ha)
|
||||
read_unlock_bh(&neigh->lock);
|
||||
}
|
||||
|
||||
static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip)
|
||||
|
Loading…
Reference in New Issue
Block a user