[RHEL7,COMMIT] netfilter: gpf inside xt_find_revision()

Submitted by Vasily Averin on March 2, 2021, 6:53 a.m.

Details

Message ID 202103020653.1226rW8B028414@vz7build.vvs.sw.ru
State New
Series "netfilter: gpf inside xt_find_revision()"
Headers show

Commit Message

Vasily Averin March 2, 2021, 6:53 a.m.
The commit is pushed to "branch-rh7-3.10.0-1160.15.2.vz7.173.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.15.2.vz7.173.1
------>
commit 2a88df439e9d8c854f140f524142da0c3433ecb9
Author: Vasily Averin <vvs@virtuozzo.com>
Date:   Tue Mar 2 09:53:32 2021 +0300

    netfilter: gpf inside xt_find_revision()
    
    nested match/target_revfn() calls work with xt[NFPROTO_UNSPEC] lists
    without taking xt[NFPROTO_UNSPEC].mutex. This can race with module unload
    and cause host to crash:
    
    general protection fault: 0000 [#1]
    Modules linked in: ... [last unloaded: xt_cluster]
    CPU: 0 PID: 542455 Comm: iptables
    RIP: 0010:[<ffffffff8ffbd518>]  [<ffffffff8ffbd518>] strcmp+0x18/0x40
    RDX: 0000000000000003 RSI: ffff9a5a5d9abe10 RDI: dead000000000111
    R13: ffff9a5a5d9abe10 R14: ffff9a5a5d9abd8c R15: dead000000000100
    (VvS: %R15 -- &xt_match,  %RDI -- &xt_match.name,
    xt_cluster unload unregister match in xt[NFPROTO_UNSPEC].match list)
    Call Trace:
     [<ffffffff902ccf44>] match_revfn+0x54/0xc0
     [<ffffffff902ccf9f>] match_revfn+0xaf/0xc0
     [<ffffffff902cd01e>] xt_find_revision+0x6e/0xf0
     [<ffffffffc05a5be0>] do_ipt_get_ctl+0x100/0x420 [ip_tables]
     [<ffffffff902cc6bf>] nf_getsockopt+0x4f/0x70
     [<ffffffff902dd99e>] ip_getsockopt+0xde/0x100
     [<ffffffff903039b5>] raw_getsockopt+0x25/0x50
     [<ffffffff9026c5da>] sock_common_getsockopt+0x1a/0x20
     [<ffffffff9026b89d>] SyS_getsockopt+0x7d/0xf0
     [<ffffffff903cbf92>] system_call_fastpath+0x25/0x2a
    
    Fixes: 656caff20e1 ("netfilter 04/09: x_tables: fix match/target revision lookup")
    https://jira.sw.ru/browse/PSBM-126137
    Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 net/netfilter/x_tables.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 1ffa1ad..ddde007 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -336,6 +336,7 @@  static int match_revfn(u8 af, const char *name, u8 revision, int *bestp)
 	const struct xt_match *m;
 	int have_rev = 0;
 
+	mutex_lock(&xt[af].mutex);
 	list_for_each_entry(m, &xt[af].match, list) {
 		if (strcmp(m->name, name) == 0) {
 			if (m->revision > *bestp)
@@ -344,6 +345,7 @@  static int match_revfn(u8 af, const char *name, u8 revision, int *bestp)
 				have_rev = 1;
 		}
 	}
+	mutex_unlock(&xt[af].mutex);
 
 	if (af != NFPROTO_UNSPEC && !have_rev)
 		return match_revfn(NFPROTO_UNSPEC, name, revision, bestp);
@@ -356,6 +358,7 @@  static int target_revfn(u8 af, const char *name, u8 revision, int *bestp)
 	const struct xt_target *t;
 	int have_rev = 0;
 
+	mutex_lock(&xt[af].mutex);
 	list_for_each_entry(t, &xt[af].target, list) {
 		if (strcmp(t->name, name) == 0) {
 			if (t->revision > *bestp)
@@ -364,6 +367,7 @@  static int target_revfn(u8 af, const char *name, u8 revision, int *bestp)
 				have_rev = 1;
 		}
 	}
+	mutex_unlock(&xt[af].mutex);
 
 	if (af != NFPROTO_UNSPEC && !have_rev)
 		return target_revfn(NFPROTO_UNSPEC, name, revision, bestp);
@@ -377,12 +381,10 @@  int xt_find_revision(u8 af, const char *name, u8 revision, int target,
 {
 	int have_rev, best = -1;
 
-	mutex_lock(&xt[af].mutex);
 	if (target == 1)
 		have_rev = target_revfn(af, name, revision, &best);
 	else
 		have_rev = match_revfn(af, name, revision, &best);
-	mutex_unlock(&xt[af].mutex);
 
 	/* Nothing at all?  Return 0 to try loading module. */
 	if (best == -1) {