| 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
| ||
|
Message-Id: <20210317185320.1561608-1-cascardo@canonical.com>
Date: Wed, 17 Mar 2021 15:53:19 -0300
From: Thadeu Lima de Souza Cascardo <cascardo@...onical.com>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net, kuba@...nel.org, dsahern@...nel.org,
Thadeu Lima de Souza Cascardo <cascardo@...onical.com>
Subject: [PATCH 1/2] neighbour: allow referenced neighbours to be removed
During forced garbage collection, neighbours with more than a reference are
not removed. It's possible to DoS the neighbour table by using ARP spoofing
in such a way that there is always a timer pending for all neighbours,
preventing any of them from being removed. That will cause any new
neighbour creation to fail.
Use the same code as used by neigh_flush_dev, which deletes the timer and
cleans the queue when there are still references left.
With the same ARP spoofing technique, it was still possible to reach a valid
destination when this fix was applied, with no more table overflows.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@...onical.com>
---
net/core/neighbour.c | 117 +++++++++++++++++++------------------------
1 file changed, 51 insertions(+), 66 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index e2982b3970b8..bbc89c7ffdfd 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -173,25 +173,48 @@ static bool neigh_update_ext_learned(struct neighbour *neigh, u32 flags,
return rc;
}
+static int neigh_del_timer(struct neighbour *n)
+{
+ if ((n->nud_state & NUD_IN_TIMER) &&
+ del_timer(&n->timer)) {
+ neigh_release(n);
+ return 1;
+ }
+ return 0;
+}
+
static bool neigh_del(struct neighbour *n, struct neighbour __rcu **np,
struct neigh_table *tbl)
{
- bool retval = false;
-
+ rcu_assign_pointer(*np,
+ rcu_dereference_protected(n->next,
+ lockdep_is_held(&tbl->lock)));
write_lock(&n->lock);
- if (refcount_read(&n->refcnt) == 1) {
- struct neighbour *neigh;
-
- neigh = rcu_dereference_protected(n->next,
- lockdep_is_held(&tbl->lock));
- rcu_assign_pointer(*np, neigh);
- neigh_mark_dead(n);
- retval = true;
+ neigh_del_timer(n);
+ neigh_mark_dead(n);
+ if (refcount_read(&n->refcnt) != 1) {
+ /* The most unpleasant situation.
+ We must destroy neighbour entry,
+ but someone still uses it.
+
+ The destroy will be delayed until
+ the last user releases us, but
+ we must kill timers etc. and move
+ it to safe state.
+ */
+ __skb_queue_purge(&n->arp_queue);
+ n->arp_queue_len_bytes = 0;
+ n->output = neigh_blackhole;
+ if (n->nud_state & NUD_VALID)
+ n->nud_state = NUD_NOARP;
+ else
+ n->nud_state = NUD_NONE;
+ neigh_dbg(2, "neigh %p is stray\n", n);
}
write_unlock(&n->lock);
- if (retval)
- neigh_cleanup_and_release(n);
- return retval;
+ neigh_cleanup_and_release(n);
+
+ return true;
}
bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl)
@@ -229,22 +252,20 @@ static int neigh_forced_gc(struct neigh_table *tbl)
write_lock_bh(&tbl->lock);
list_for_each_entry_safe(n, tmp, &tbl->gc_list, gc_list) {
- if (refcount_read(&n->refcnt) == 1) {
- bool remove = false;
-
- write_lock(&n->lock);
- if ((n->nud_state == NUD_FAILED) ||
- (tbl->is_multicast &&
- tbl->is_multicast(n->primary_key)) ||
- time_after(tref, n->updated))
- remove = true;
- write_unlock(&n->lock);
-
- if (remove && neigh_remove_one(n, tbl))
- shrunk++;
- if (shrunk >= max_clean)
- break;
- }
+ bool remove = false;
+
+ write_lock(&n->lock);
+ if ((n->nud_state == NUD_FAILED) ||
+ (tbl->is_multicast &&
+ tbl->is_multicast(n->primary_key)) ||
+ time_after(tref, n->updated))
+ remove = true;
+ write_unlock(&n->lock);
+
+ if (remove && neigh_remove_one(n, tbl))
+ shrunk++;
+ if (shrunk >= max_clean)
+ break;
}
tbl->last_flush = jiffies;
@@ -264,16 +285,6 @@ static void neigh_add_timer(struct neighbour *n, unsigned long when)
}
}
-static int neigh_del_timer(struct neighbour *n)
-{
- if ((n->nud_state & NUD_IN_TIMER) &&
- del_timer(&n->timer)) {
- neigh_release(n);
- return 1;
- }
- return 0;
-}
-
static void pneigh_queue_purge(struct sk_buff_head *list)
{
struct sk_buff *skb;
@@ -307,33 +318,7 @@ static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev,
np = &n->next;
continue;
}
- rcu_assign_pointer(*np,
- rcu_dereference_protected(n->next,
- lockdep_is_held(&tbl->lock)));
- write_lock(&n->lock);
- neigh_del_timer(n);
- neigh_mark_dead(n);
- if (refcount_read(&n->refcnt) != 1) {
- /* The most unpleasant situation.
- We must destroy neighbour entry,
- but someone still uses it.
-
- The destroy will be delayed until
- the last user releases us, but
- we must kill timers etc. and move
- it to safe state.
- */
- __skb_queue_purge(&n->arp_queue);
- n->arp_queue_len_bytes = 0;
- n->output = neigh_blackhole;
- if (n->nud_state & NUD_VALID)
- n->nud_state = NUD_NOARP;
- else
- n->nud_state = NUD_NONE;
- neigh_dbg(2, "neigh %p is stray\n", n);
- }
- write_unlock(&n->lock);
- neigh_cleanup_and_release(n);
+ neigh_del(n, np, tbl);
}
}
}
--
2.27.0
Powered by blists - more mailing lists