From 3c9e9f7320f0138497ef7879c0903246746e0ed3 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Thu, 12 Mar 2015 14:46:23 -0700 Subject: [PATCH 1/2] fib_trie: Avoid NULL pointer if local table is not allocated The function fib_unmerge assumed the local table had already been allocated. If that is not the case however when custom rules are applied then this can result in a NULL pointer dereference. In order to prevent this we must check the value of the local table pointer and if it is NULL simply return 0 as there is no local table to separate from the main. Fixes: 0ddcf43d5 ("ipv4: FIB Local/MAIN table collapse") Reported-by: Madhu Challa Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- net/ipv4/fib_frontend.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index c1caf9ded280c..e5b6b0534c5f5 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -156,9 +156,12 @@ int fib_unmerge(struct net *net) { struct fib_table *old, *new; + /* attempt to fetch local table if it has been allocated */ old = fib_get_table(net, RT_TABLE_LOCAL); - new = fib_trie_unmerge(old); + if (!old) + return 0; + new = fib_trie_unmerge(old); if (!new) return -ENOMEM; From 0b65bd97ba5fc2c43fa4d077e7420f3ec09a40b3 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Thu, 12 Mar 2015 14:46:29 -0700 Subject: [PATCH 2/2] fib_trie: Provide a deterministic order for fib_alias w/ tables merged This change makes it so that we should always have a deterministic ordering for the main and local aliases within the merged table when two leaves overlap. So for example if we have a leaf with a key of 192.168.254.0. If we previously added two aliases with a prefix length of 24 from both local and main the first entry would be first and the second would be second. When I was coding this I had added a WARN_ON should such a situation occur as I wasn't sure how likely it would be. However this WARN_ON has been triggered so this is something that should be addressed. With this patch the ordering of the aliases is as follows. First they are sorted on prefix length, then on their table ID, then tos, and finally priority. This way what we end up doing is essentially interleaving the two tables on what used to be leaf_info structure boundaries. Fixes: 0ddcf43d5 ("ipv4: FIB Local/MAIN table collapse") Reported-by: Eric Dumazet Signed-off-by: Alexander Duyck Signed-off-by: David S. Miller --- net/ipv4/fib_trie.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c index dd488c102d899..e3b4aee4244e1 100644 --- a/net/ipv4/fib_trie.c +++ b/net/ipv4/fib_trie.c @@ -950,7 +950,7 @@ static struct key_vector *fib_find_node(struct trie *t, * priority less than or equal to PRIO. */ static struct fib_alias *fib_find_alias(struct hlist_head *fah, u8 slen, - u8 tos, u32 prio) + u8 tos, u32 prio, u32 tb_id) { struct fib_alias *fa; @@ -962,6 +962,10 @@ static struct fib_alias *fib_find_alias(struct hlist_head *fah, u8 slen, continue; if (fa->fa_slen != slen) break; + if (fa->tb_id > tb_id) + continue; + if (fa->tb_id != tb_id) + break; if (fa->fa_tos > tos) continue; if (fa->fa_info->fib_priority >= prio || fa->fa_tos < tos) @@ -1041,6 +1045,9 @@ static int fib_insert_alias(struct trie *t, struct key_vector *tp, hlist_for_each_entry(last, &l->leaf, fa_list) { if (new->fa_slen < last->fa_slen) break; + if ((new->fa_slen == last->fa_slen) && + (new->tb_id > last->tb_id)) + break; fa = last; } @@ -1089,7 +1096,8 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg) } l = fib_find_node(t, &tp, key); - fa = l ? fib_find_alias(&l->leaf, slen, tos, fi->fib_priority) : NULL; + fa = l ? fib_find_alias(&l->leaf, slen, tos, fi->fib_priority, + tb->tb_id) : NULL; /* Now fa, if non-NULL, points to the first fib alias * with the same keys [prefix,tos,priority], if such key already @@ -1116,13 +1124,12 @@ int fib_table_insert(struct fib_table *tb, struct fib_config *cfg) fa_match = NULL; fa_first = fa; hlist_for_each_entry_from(fa, fa_list) { - if ((fa->fa_slen != slen) || (fa->fa_tos != tos)) + if ((fa->fa_slen != slen) || + (fa->tb_id != tb->tb_id) || + (fa->fa_tos != tos)) break; if (fa->fa_info->fib_priority != fi->fib_priority) break; - /* duplicate entry from another table */ - if (WARN_ON(fa->tb_id != tb->tb_id)) - continue; if (fa->fa_type == cfg->fc_type && fa->fa_info == fi) { fa_match = fa; @@ -1474,7 +1481,7 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg) if (!l) return -ESRCH; - fa = fib_find_alias(&l->leaf, slen, tos, 0); + fa = fib_find_alias(&l->leaf, slen, tos, 0, tb->tb_id); if (!fa) return -ESRCH; @@ -1484,12 +1491,11 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg) hlist_for_each_entry_from(fa, fa_list) { struct fib_info *fi = fa->fa_info; - if ((fa->fa_slen != slen) || (fa->fa_tos != tos)) + if ((fa->fa_slen != slen) || + (fa->tb_id != tb->tb_id) || + (fa->fa_tos != tos)) break; - if (fa->tb_id != tb->tb_id) - continue; - if ((!cfg->fc_type || fa->fa_type == cfg->fc_type) && (cfg->fc_scope == RT_SCOPE_NOWHERE || fa->fa_info->fib_scope == cfg->fc_scope) &&