Commit:     69cc64d8d92bf852f933e90c888dfff083bd4fc9
Parent:     3611f4d2a5e0f6135805f88bc5ecb63fa9ee5107
Author:     David S. Miller <[EMAIL PROTECTED]>
AuthorDate: Mon Feb 11 21:45:44 2008 -0800
Committer:  David S. Miller <[EMAIL PROTECTED]>
CommitDate: Tue Feb 12 17:54:17 2008 -0800

    [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 <[EMAIL PROTECTED]>
 net/core/neighbour.c |   12 +++---------
 net/ipv4/arp.c       |    3 ---
 2 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index a16cf1e..7bb6a9a 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -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);
-               if (skb)
-                       kfree_skb(skb);
-       } else {
-               write_unlock(&neigh->lock);
+       write_unlock(&neigh->lock);
        if (notify)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 8e17f65..c663fa5 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -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 
                dst_ha = neigh->ha;
-               read_lock_bh(&neigh->lock);
        } else if ((probes -= neigh->parms->app_probes) < 0) {
@@ -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)
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at

Reply via email to