Skip to content

Commit 67d6d68

Browse files
edumazetdavem330
authored andcommitted
ipv4: make exception cache less predictible
Even after commit 6457378 ("ipv4: use siphash instead of Jenkins in fnhe_hashfun()"), an attacker can still use brute force to learn some secrets from a victim linux host. One way to defeat these attacks is to make the max depth of the hash table bucket a random value. Before this patch, each bucket of the hash table used to store exceptions could contain 6 items under attack. After the patch, each bucket would contains a random number of items, between 6 and 10. The attacker can no longer infer secrets. This is slightly increasing memory size used by the hash table, by 50% in average, we do not expect this to be a problem. This patch is more complex than the prior one (IPv6 equivalent), because IPv4 was reusing the oldest entry. Since we need to be able to evict more than one entry per update_or_create_fnhe() call, I had to replace fnhe_oldest() with fnhe_remove_oldest(). Also note that we will queue extra kfree_rcu() calls under stress, which hopefully wont be a too big issue. Fixes: 4895c77 ("ipv4: Add FIB nexthop exceptions.") Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: Keyu Man <kman001@ucr.edu> Cc: Willy Tarreau <w@1wt.eu> Signed-off-by: David S. Miller <davem@davemloft.net> Reviewed-by: David Ahern <dsahern@kernel.org> Tested-by: David Ahern <dsahern@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent a00df2c commit 67d6d68

File tree

1 file changed

+30
-16
lines changed

1 file changed

+30
-16
lines changed

net/ipv4/route.c

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -587,18 +587,25 @@ static void fnhe_flush_routes(struct fib_nh_exception *fnhe)
587587
}
588588
}
589589

590-
static struct fib_nh_exception *fnhe_oldest(struct fnhe_hash_bucket *hash)
590+
static void fnhe_remove_oldest(struct fnhe_hash_bucket *hash)
591591
{
592-
struct fib_nh_exception *fnhe, *oldest;
592+
struct fib_nh_exception __rcu **fnhe_p, **oldest_p;
593+
struct fib_nh_exception *fnhe, *oldest = NULL;
593594

594-
oldest = rcu_dereference(hash->chain);
595-
for (fnhe = rcu_dereference(oldest->fnhe_next); fnhe;
596-
fnhe = rcu_dereference(fnhe->fnhe_next)) {
597-
if (time_before(fnhe->fnhe_stamp, oldest->fnhe_stamp))
595+
for (fnhe_p = &hash->chain; ; fnhe_p = &fnhe->fnhe_next) {
596+
fnhe = rcu_dereference_protected(*fnhe_p,
597+
lockdep_is_held(&fnhe_lock));
598+
if (!fnhe)
599+
break;
600+
if (!oldest ||
601+
time_before(fnhe->fnhe_stamp, oldest->fnhe_stamp)) {
598602
oldest = fnhe;
603+
oldest_p = fnhe_p;
604+
}
599605
}
600606
fnhe_flush_routes(oldest);
601-
return oldest;
607+
*oldest_p = oldest->fnhe_next;
608+
kfree_rcu(oldest, rcu);
602609
}
603610

604611
static u32 fnhe_hashfun(__be32 daddr)
@@ -677,23 +684,30 @@ static void update_or_create_fnhe(struct fib_nh_common *nhc, __be32 daddr,
677684
if (rt)
678685
fill_route_from_fnhe(rt, fnhe);
679686
} else {
680-
if (depth > FNHE_RECLAIM_DEPTH)
681-
fnhe = fnhe_oldest(hash);
682-
else {
683-
fnhe = kzalloc(sizeof(*fnhe), GFP_ATOMIC);
684-
if (!fnhe)
685-
goto out_unlock;
686-
687-
fnhe->fnhe_next = hash->chain;
688-
rcu_assign_pointer(hash->chain, fnhe);
687+
/* Randomize max depth to avoid some side channels attacks. */
688+
int max_depth = FNHE_RECLAIM_DEPTH +
689+
prandom_u32_max(FNHE_RECLAIM_DEPTH);
690+
691+
while (depth > max_depth) {
692+
fnhe_remove_oldest(hash);
693+
depth--;
689694
}
695+
696+
fnhe = kzalloc(sizeof(*fnhe), GFP_ATOMIC);
697+
if (!fnhe)
698+
goto out_unlock;
699+
700+
fnhe->fnhe_next = hash->chain;
701+
690702
fnhe->fnhe_genid = genid;
691703
fnhe->fnhe_daddr = daddr;
692704
fnhe->fnhe_gw = gw;
693705
fnhe->fnhe_pmtu = pmtu;
694706
fnhe->fnhe_mtu_locked = lock;
695707
fnhe->fnhe_expires = max(1UL, expires);
696708

709+
rcu_assign_pointer(hash->chain, fnhe);
710+
697711
/* Exception created; mark the cached routes for the nexthop
698712
* stale, so anyone caching it rechecks if this exception
699713
* applies to them.

0 commit comments

Comments
 (0)