Skip to content

Commit 24a76a1

Browse files
Florian Westphalroidayan
authored andcommitted
netfilter: conntrack: fix lookup race during hash resize
When resizing the conntrack hash table at runtime via echo 42 > /sys/module/nf_conntrack/parameters/hashsize, we are racing with the conntrack lookup path -- reads can happen in parallel and nothing prevents readers from observing a the newly allocated hash but the old size (or vice versa). So access to hash[bucket] can trigger OOB read access in case the table got expanded and we saw the new size but the old hash pointer (or it got shrunk and we got new hash ptr but the size of the old and larger table): kasan: GPF could be caused by NULL-ptr deref or user memory access general protection fault: 0000 [#1] SMP KASAN CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 4.6.0-rc2+ torvalds#107 [..] Call Trace: [<ffffffff822c3d6a>] ? nf_conntrack_tuple_taken+0x12a/0xe90 [<ffffffff822c3ac1>] ? nf_ct_invert_tuplepr+0x221/0x3a0 [<ffffffff8230e703>] get_unique_tuple+0xfb3/0x2760 Use generation counter to obtain the address/length of the same table. Also add a synchronize_net before freeing the old hash. AFAICS, without it we might access ct_hash[bucket] after ct_hash has been freed, provided that lockless reader got delayed by another event: CPU1 CPU2 seq_begin seq_retry <delay> resize occurs free oldhash for_each(oldhash[size]) Note that resize is only supported in init_netns, it took over 2 minutes of constant resizing+flooding to produce the warning, so this isn't a big problem in practice. Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent 9635c6c commit 24a76a1

File tree

1 file changed

+19
-5
lines changed

1 file changed

+19
-5
lines changed

net/netfilter/nf_conntrack_core.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -501,11 +501,18 @@ ____nf_conntrack_find(struct net *net, const struct nf_conntrack_zone *zone,
501501
const struct nf_conntrack_tuple *tuple, u32 hash)
502502
{
503503
struct nf_conntrack_tuple_hash *h;
504+
struct hlist_nulls_head *ct_hash;
504505
struct hlist_nulls_node *n;
505-
unsigned int bucket = hash_bucket(hash, net);
506+
unsigned int bucket, sequence;
506507

507508
begin:
508-
hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[bucket], hnnode) {
509+
do {
510+
sequence = read_seqcount_begin(&nf_conntrack_generation);
511+
bucket = hash_bucket(hash, net);
512+
ct_hash = net->ct.hash;
513+
} while (read_seqcount_retry(&nf_conntrack_generation, sequence));
514+
515+
hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[bucket], hnnode) {
509516
if (nf_ct_key_equal(h, tuple, zone)) {
510517
NF_CT_STAT_INC_ATOMIC(net, found);
511518
return h;
@@ -754,15 +761,21 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
754761
struct net *net = nf_ct_net(ignored_conntrack);
755762
const struct nf_conntrack_zone *zone;
756763
struct nf_conntrack_tuple_hash *h;
764+
struct hlist_nulls_head *ct_hash;
765+
unsigned int hash, sequence;
757766
struct hlist_nulls_node *n;
758767
struct nf_conn *ct;
759-
unsigned int hash;
760768

761769
zone = nf_ct_zone(ignored_conntrack);
762-
hash = hash_conntrack(net, tuple);
763770

764771
rcu_read_lock();
765-
hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[hash], hnnode) {
772+
do {
773+
sequence = read_seqcount_begin(&nf_conntrack_generation);
774+
hash = hash_conntrack(net, tuple);
775+
ct_hash = net->ct.hash;
776+
} while (read_seqcount_retry(&nf_conntrack_generation, sequence));
777+
778+
hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[hash], hnnode) {
766779
ct = nf_ct_tuplehash_to_ctrack(h);
767780
if (ct != ignored_conntrack &&
768781
nf_ct_tuple_equal(tuple, &h->tuple) &&
@@ -1689,6 +1702,7 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
16891702
nf_conntrack_all_unlock();
16901703
local_bh_enable();
16911704

1705+
synchronize_net();
16921706
nf_ct_free_hashtable(old_hash, old_size);
16931707
return 0;
16941708
}

0 commit comments

Comments
 (0)