netdev - Issue with gratuitous arps when new addr is different from cached addr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAO7SqHAVz2D53u141Za01DvBFnLky2imkYNvBV-2UuALpC3w6w@mail.gmail.com>
Date:	Wed, 20 Nov 2013 16:40:52 -0800
From:	Salam Noureddine <noureddine@...stanetworks.com>
To:	"David S. Miller" <davem@...emloft.net>,
	Daniel Borkmann <dborkman@...hat.com>,
	Willem de Bruijn <willemb@...gle.com>,
	Phil Sutter <phil@....cc>, Eric Dumazet <edumazet@...gle.com>,
	netdev@...r.kernel.org
Subject: Issue with gratuitous arps when new addr is different from cached addr

Hi,

It seems to me that neigh_update is not handling correctly the case
when the new address is different from the cached one and
NEIGH_UPDATE_F_OVERRIDE is not set. When we receive a gratuitous arp
request we check jiffies against the neigh->updated + locktime in
arp_process. If we're passed that time then the flag is set.

In neigh_update, we set neigh->updated before checking for the case
where we have a new address and the override flag is not set. This
means, that we "extend the life of the old address". By setting
locktime to 2 sec and sending an arp with a new address every 1 sec, I
was able to perpetuate the old entry for as long as I wanted.

To fix this, we can just move setting neigh->updated to after the
check for new address and override flag not present,

--- linux-3.4.orig/net/core/neighbour.c
+++ linux-3.4/net/core/neighbour.c
@@ -1206,10 +1206,6 @@ int neigh_update(struct neighbour *neigh
                lladdr = neigh->ha;
        }

-       if (new & NUD_CONNECTED)
-               neigh->confirmed = jiffies;
-       neigh->updated = jiffies;
-
        /* If entry was valid and address is not changed,
           do not change entry state, if new one is STALE.
         */
@@ -1233,6 +1229,10 @@ int neigh_update(struct neighbour *neigh
                }
        }

+       if (new & NUD_CONNECTED)
+               neigh->confirmed = jiffies;
+       neigh->updated = jiffies;
+
        if (new != old) {
                neigh_del_timer(neigh);
                if (new & NUD_IN_TIMER)

If that seems like a an acceptable solution, I would post a patch shortly.

Thanks,

Salam
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html