Skip to content

Commit

Permalink
neigh: do not modify unlinked entries
Browse files Browse the repository at this point in the history
The lockless lookups can return entry that is unlinked.
Sometimes they get reference before last neigh_cleanup_and_release,
sometimes they do not need reference. Later, any
modification attempts may result in the following problems:

1. entry is not destroyed immediately because neigh_update
can start the timer for dead entry, eg. on change to NUD_REACHABLE
state. As result, entry lives for some time but is invisible
and out of control.

2. __neigh_event_send can run in parallel with neigh_destroy
while refcnt=0 but if timer is started and expired refcnt can
reach 0 for second time leading to second neigh_destroy and
possible crash.

Thanks to Eric Dumazet and Ying Xue for their work and analyze
on the __neigh_event_send change.

Fixes: 767e97e ("neigh: RCU conversion of struct neighbour")
Fixes: a263b30 ("ipv4: Make neigh lookups directly in output packet path.")
Fixes: 6fd6ce2 ("ipv6: Do not depend on rt->n in ip6_finish_output2().")
Cc: Eric Dumazet <[email protected]>
Cc: Ying Xue <[email protected]>
Signed-off-by: Julian Anastasov <[email protected]>
Acked-by: Eric Dumazet <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
Julian Anastasov authored and davem330 committed Jun 21, 2015
1 parent f98f451 commit 2c51a97
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions net/core/neighbour.c
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,8 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
rc = 0;
if (neigh->nud_state & (NUD_CONNECTED | NUD_DELAY | NUD_PROBE))
goto out_unlock_bh;
if (neigh->dead)
goto out_dead;

if (!(neigh->nud_state & (NUD_STALE | NUD_INCOMPLETE))) {
if (NEIGH_VAR(neigh->parms, MCAST_PROBES) +
Expand Down Expand Up @@ -1013,6 +1015,13 @@ int __neigh_event_send(struct neighbour *neigh, struct sk_buff *skb)
write_unlock(&neigh->lock);
local_bh_enable();
return rc;

out_dead:
if (neigh->nud_state & NUD_STALE)
goto out_unlock_bh;
write_unlock_bh(&neigh->lock);
kfree_skb(skb);
return 1;
}
EXPORT_SYMBOL(__neigh_event_send);

Expand Down Expand Up @@ -1076,6 +1085,8 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
if (!(flags & NEIGH_UPDATE_F_ADMIN) &&
(old & (NUD_NOARP | NUD_PERMANENT)))
goto out;
if (neigh->dead)
goto out;

if (!(new & NUD_VALID)) {
neigh_del_timer(neigh);
Expand Down Expand Up @@ -1225,6 +1236,8 @@ EXPORT_SYMBOL(neigh_update);
*/
void __neigh_set_probe_once(struct neighbour *neigh)
{
if (neigh->dead)
return;
neigh->updated = jiffies;
if (!(neigh->nud_state & NUD_FAILED))
return;
Expand Down

0 comments on commit 2c51a97

Please sign in to comment.