[RHEL7,COMMIT] nf_conntrack: fix possible use-after-free on nf_conntrack module reloading.

Submitted by Konstantin Khorenko on June 18, 2018, 12:29 p.m.

Details

Message ID 201806181229.w5ICTPV0021688@finist_ce7.work
State New
Series "nf_conntrack: fix possible use-after-free on nf_conntrack module reloading."
Headers show

Commit Message

Konstantin Khorenko June 18, 2018, 12:29 p.m.
The commit is pushed to "branch-rh7-3.10.0-693.21.1.vz7.50.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-693.21.1.vz7.50.12
------>
commit ae099bfe52867b2a799c493bfdccdb9fc41c8b2a
Author: Andrey Ryabinin <aryabinin@virtuozzo.com>
Date:   Mon Jun 18 15:29:25 2018 +0300

    nf_conntrack: fix possible use-after-free on nf_conntrack module reloading.
    
    Reloading nf_conntrack module with doubled hashsize parameter, i.e.
              rmmod nf_conntrack
              modprobe nf_conntrack hashsize=12345 hashsize=12345
    causes use-after-free.
    
    On module unload, nf_conntrack_cleanup_net_list() frees init_net.ct.hash.
    On the next module loading nf_conntrack_set_hashsize() called twice,
    because hashsize parameter specified twice.
    On the first nf_conntrack_set_hashsize() call we just set
    'nf_conntrack_htable_size' variable:
    
            nf_conntrack_set_hashsize()
                    ...
                    /* On boot, we can set this without any fancy locking. */
                    if (!nf_conntrack_htable_size)
                            return param_set_uint(val, kp);
    
    But on the second invocation, nf_conntrack_htable_size is already set,
    so we pass further, use already freed init_net.ct.hash hashtable and
    free it again at the end:
            ...
            old_size = init_net.ct.htable_size;
            old_hash = init_net.ct.hash;
    
            init_net.ct.htable_size = nf_conntrack_htable_size = hashsize;
            init_net.ct.hash = hash;
    
            ....
            nf_ct_free_hashtable(old_hash, old_size);
    
    Fix this, by checking !init_net.ct.htable_size instead of
    !nf_conntrack_htable_size. init_net.ct.htable_size will be initialized
    only in init callback, so on the second invocation of the
    nf_conntrack_set_hashsize() will just reinitialize nf_conntrack_htable_size
    and nothing more.
    Also set to zero init_net.ct on module removal, since init_net.ct.htable_size
    should be zero on next module loading.
    
    https://pmc.acronis.com/browse/VSTOR-11099
    Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 net/netfilter/nf_conntrack_core.c       | 2 +-
 net/netfilter/nf_conntrack_standalone.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 2337c1774a5d..8b3cec16ceb3 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1629,7 +1629,7 @@  int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
 		return -EOPNOTSUPP;
 
 	/* On boot, we can set this without any fancy locking. */
-	if (!nf_conntrack_htable_size)
+	if (!init_net.ct.htable_size)
 		return param_set_uint(val, kp);
 
 	rc = kstrtouint(val, 0, &hashsize);
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index aa23d68d146f..86fea808ade3 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -693,6 +693,12 @@  static void __exit nf_conntrack_standalone_fini(void)
 {
 	nf_conntrack_cleanup_start();
 	unregister_pernet_subsys(&nf_conntrack_net_ops);
+
+	/*
+	 * Make sure on the next module load we will not use dangling
+	 * pointers.
+	 */
+	memset(&init_net.ct, 0, sizeof(init_net.ct));
 	nf_conntrack_cleanup_end();
 }