You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
352 lines
11 KiB
352 lines
11 KiB
![]()
5 years ago
|
From 5e9e2dc7e972f6bbbc0156ad97b4ee9d11fcb837 Mon Sep 17 00:00:00 2001
|
||
|
From: Phil Sutter <psutter@redhat.com>
|
||
|
Date: Wed, 20 Jun 2018 09:22:47 +0200
|
||
|
Subject: [PATCH] netlink_delinearize: Fix resource leaks
|
||
|
|
||
|
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1504157
|
||
|
Upstream Status: nftables commit 671851617c8d8
|
||
|
|
||
|
commit 671851617c8d8c1dfe9822eee8dcc7b827fff850
|
||
|
Author: Phil Sutter <phil@nwl.cc>
|
||
|
Date: Thu Mar 1 15:00:32 2018 +0100
|
||
|
|
||
|
netlink_delinearize: Fix resource leaks
|
||
|
|
||
|
Most of the cases are basically the same: Error path fails to free the
|
||
|
previously allocated statement or expression. A few cases received
|
||
|
special treatment though:
|
||
|
|
||
|
- In netlink_parse_payload_stmt(), the leak is easily avoided by code
|
||
|
reordering.
|
||
|
|
||
|
- In netlink_parse_exthdr(), there's no point in introducing a goto
|
||
|
label since there is but a single affected error check.
|
||
|
|
||
|
- In netlink_parse_hash() non-error path leaked as well if sreg
|
||
|
contained a concatenated expression.
|
||
|
|
||
|
Signed-off-by: Phil Sutter <phil@nwl.cc>
|
||
|
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
||
|
---
|
||
|
src/netlink_delinearize.c | 144 +++++++++++++++++++++++++++++-----------------
|
||
|
1 file changed, 92 insertions(+), 52 deletions(-)
|
||
|
|
||
|
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
|
||
|
index 61cba52..e25160a 100644
|
||
|
--- a/src/netlink_delinearize.c
|
||
|
+++ b/src/netlink_delinearize.c
|
||
|
@@ -470,15 +470,15 @@ static void netlink_parse_payload_stmt(struct netlink_parse_ctx *ctx,
|
||
|
offset = nftnl_expr_get_u32(nle, NFTNL_EXPR_PAYLOAD_OFFSET) * BITS_PER_BYTE;
|
||
|
len = nftnl_expr_get_u32(nle, NFTNL_EXPR_PAYLOAD_LEN) * BITS_PER_BYTE;
|
||
|
|
||
|
- expr = payload_expr_alloc(loc, NULL, 0);
|
||
|
- payload_init_raw(expr, base, offset, len);
|
||
|
-
|
||
|
sreg = netlink_parse_register(nle, NFTNL_EXPR_PAYLOAD_SREG);
|
||
|
val = netlink_get_register(ctx, loc, sreg);
|
||
|
if (val == NULL)
|
||
|
return netlink_error(ctx, loc,
|
||
|
"payload statement has no expression");
|
||
|
|
||
|
+ expr = payload_expr_alloc(loc, NULL, 0);
|
||
|
+ payload_init_raw(expr, base, offset, len);
|
||
|
+
|
||
|
stmt = payload_stmt_alloc(loc, expr, val);
|
||
|
|
||
|
list_add_tail(&stmt->list, &ctx->rule->stmts);
|
||
|
@@ -523,9 +523,11 @@ static void netlink_parse_exthdr(struct netlink_parse_ctx *ctx,
|
||
|
|
||
|
sreg = netlink_parse_register(nle, NFTNL_EXPR_EXTHDR_SREG);
|
||
|
val = netlink_get_register(ctx, loc, sreg);
|
||
|
- if (val == NULL)
|
||
|
+ if (val == NULL) {
|
||
|
+ xfree(expr);
|
||
|
return netlink_error(ctx, loc,
|
||
|
"exthdr statement has no expression");
|
||
|
+ }
|
||
|
|
||
|
expr_set_type(val, expr->dtype, expr->byteorder);
|
||
|
|
||
|
@@ -556,22 +558,27 @@ static void netlink_parse_hash(struct netlink_parse_ctx *ctx,
|
||
|
sreg = netlink_parse_register(nle, NFTNL_EXPR_HASH_SREG);
|
||
|
hexpr = netlink_get_register(ctx, loc, sreg);
|
||
|
|
||
|
- if (hexpr == NULL)
|
||
|
- return
|
||
|
+ if (hexpr == NULL) {
|
||
|
netlink_error(ctx, loc,
|
||
|
"hash statement has no expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
len = nftnl_expr_get_u32(nle,
|
||
|
NFTNL_EXPR_HASH_LEN) * BITS_PER_BYTE;
|
||
|
if (hexpr->len < len) {
|
||
|
+ xfree(hexpr);
|
||
|
hexpr = netlink_parse_concat_expr(ctx, loc, sreg, len);
|
||
|
if (hexpr == NULL)
|
||
|
- return;
|
||
|
+ goto out_err;
|
||
|
}
|
||
|
expr->hash.expr = hexpr;
|
||
|
}
|
||
|
|
||
|
dreg = netlink_parse_register(nle, NFTNL_EXPR_HASH_DREG);
|
||
|
netlink_set_register(ctx, dreg, expr);
|
||
|
+ return;
|
||
|
+out_err:
|
||
|
+ xfree(expr);
|
||
|
}
|
||
|
|
||
|
static void netlink_parse_fib(struct netlink_parse_ctx *ctx,
|
||
|
@@ -853,10 +860,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,
|
||
|
reg1 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_ADDR_MIN);
|
||
|
if (reg1) {
|
||
|
addr = netlink_get_register(ctx, loc, reg1);
|
||
|
- if (addr == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "NAT statement has no address "
|
||
|
- "expression");
|
||
|
+ if (addr == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "NAT statement has no address expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
if (family == AF_INET)
|
||
|
expr_set_type(addr, &ipaddr_type, BYTEORDER_BIG_ENDIAN);
|
||
|
@@ -869,10 +877,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,
|
||
|
reg2 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_ADDR_MAX);
|
||
|
if (reg2 && reg2 != reg1) {
|
||
|
addr = netlink_get_register(ctx, loc, reg2);
|
||
|
- if (addr == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "NAT statement has no address "
|
||
|
- "expression");
|
||
|
+ if (addr == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "NAT statement has no address expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
if (family == AF_INET)
|
||
|
expr_set_type(addr, &ipaddr_type, BYTEORDER_BIG_ENDIAN);
|
||
|
@@ -887,10 +896,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,
|
||
|
reg1 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_PROTO_MIN);
|
||
|
if (reg1) {
|
||
|
proto = netlink_get_register(ctx, loc, reg1);
|
||
|
- if (proto == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "NAT statement has no proto "
|
||
|
- "expression");
|
||
|
+ if (proto == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "NAT statement has no proto expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);
|
||
|
stmt->nat.proto = proto;
|
||
|
@@ -899,10 +909,11 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,
|
||
|
reg2 = netlink_parse_register(nle, NFTNL_EXPR_NAT_REG_PROTO_MAX);
|
||
|
if (reg2 && reg2 != reg1) {
|
||
|
proto = netlink_get_register(ctx, loc, reg2);
|
||
|
- if (proto == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "NAT statement has no proto "
|
||
|
- "expression");
|
||
|
+ if (proto == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "NAT statement has no proto expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);
|
||
|
if (stmt->nat.proto != NULL)
|
||
|
@@ -911,6 +922,9 @@ static void netlink_parse_nat(struct netlink_parse_ctx *ctx,
|
||
|
}
|
||
|
|
||
|
ctx->stmt = stmt;
|
||
|
+ return;
|
||
|
+out_err:
|
||
|
+ xfree(stmt);
|
||
|
}
|
||
|
|
||
|
static void netlink_parse_masq(struct netlink_parse_ctx *ctx,
|
||
|
@@ -931,10 +945,11 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,
|
||
|
reg1 = netlink_parse_register(nle, NFTNL_EXPR_MASQ_REG_PROTO_MIN);
|
||
|
if (reg1) {
|
||
|
proto = netlink_get_register(ctx, loc, reg1);
|
||
|
- if (proto == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "MASQUERADE statement"
|
||
|
- "has no proto expression");
|
||
|
+ if (proto == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "MASQUERADE statement has no proto expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);
|
||
|
stmt->masq.proto = proto;
|
||
|
}
|
||
|
@@ -942,10 +957,11 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,
|
||
|
reg2 = netlink_parse_register(nle, NFTNL_EXPR_MASQ_REG_PROTO_MAX);
|
||
|
if (reg2 && reg2 != reg1) {
|
||
|
proto = netlink_get_register(ctx, loc, reg2);
|
||
|
- if (proto == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "MASQUERADE statement"
|
||
|
- "has no proto expression");
|
||
|
+ if (proto == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "MASQUERADE statement has no proto expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);
|
||
|
if (stmt->masq.proto != NULL)
|
||
|
proto = range_expr_alloc(loc, stmt->masq.proto, proto);
|
||
|
@@ -953,6 +969,9 @@ static void netlink_parse_masq(struct netlink_parse_ctx *ctx,
|
||
|
}
|
||
|
|
||
|
ctx->stmt = stmt;
|
||
|
+ return;
|
||
|
+out_err:
|
||
|
+ xfree(stmt);
|
||
|
}
|
||
|
|
||
|
static void netlink_parse_redir(struct netlink_parse_ctx *ctx,
|
||
|
@@ -974,10 +993,11 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,
|
||
|
reg1 = netlink_parse_register(nle, NFTNL_EXPR_REDIR_REG_PROTO_MIN);
|
||
|
if (reg1) {
|
||
|
proto = netlink_get_register(ctx, loc, reg1);
|
||
|
- if (proto == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "redirect statement has no proto "
|
||
|
- "expression");
|
||
|
+ if (proto == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "redirect statement has no proto expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);
|
||
|
stmt->redir.proto = proto;
|
||
|
@@ -986,10 +1006,11 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,
|
||
|
reg2 = netlink_parse_register(nle, NFTNL_EXPR_REDIR_REG_PROTO_MAX);
|
||
|
if (reg2 && reg2 != reg1) {
|
||
|
proto = netlink_get_register(ctx, loc, reg2);
|
||
|
- if (proto == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "redirect statement has no proto "
|
||
|
- "expression");
|
||
|
+ if (proto == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "redirect statement has no proto expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
expr_set_type(proto, &inet_service_type, BYTEORDER_BIG_ENDIAN);
|
||
|
if (stmt->redir.proto != NULL)
|
||
|
@@ -999,6 +1020,9 @@ static void netlink_parse_redir(struct netlink_parse_ctx *ctx,
|
||
|
}
|
||
|
|
||
|
ctx->stmt = stmt;
|
||
|
+ return;
|
||
|
+out_err:
|
||
|
+ xfree(stmt);
|
||
|
}
|
||
|
|
||
|
static void netlink_parse_dup(struct netlink_parse_ctx *ctx,
|
||
|
@@ -1014,9 +1038,11 @@ static void netlink_parse_dup(struct netlink_parse_ctx *ctx,
|
||
|
reg1 = netlink_parse_register(nle, NFTNL_EXPR_DUP_SREG_ADDR);
|
||
|
if (reg1) {
|
||
|
addr = netlink_get_register(ctx, loc, reg1);
|
||
|
- if (addr == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "DUP statement has no destination expression");
|
||
|
+ if (addr == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "DUP statement has no destination expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
switch (ctx->table->handle.family) {
|
||
|
case NFPROTO_IPV4:
|
||
|
@@ -1033,9 +1059,11 @@ static void netlink_parse_dup(struct netlink_parse_ctx *ctx,
|
||
|
reg2 = netlink_parse_register(nle, NFTNL_EXPR_DUP_SREG_DEV);
|
||
|
if (reg2) {
|
||
|
dev = netlink_get_register(ctx, loc, reg2);
|
||
|
- if (dev == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "DUP statement has no output expression");
|
||
|
+ if (dev == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "DUP statement has no output expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
expr_set_type(dev, &ifindex_type, BYTEORDER_HOST_ENDIAN);
|
||
|
if (stmt->dup.to == NULL)
|
||
|
@@ -1045,6 +1073,9 @@ static void netlink_parse_dup(struct netlink_parse_ctx *ctx,
|
||
|
}
|
||
|
|
||
|
ctx->stmt = stmt;
|
||
|
+ return;
|
||
|
+out_err:
|
||
|
+ xfree(stmt);
|
||
|
}
|
||
|
|
||
|
static void netlink_parse_fwd(struct netlink_parse_ctx *ctx,
|
||
|
@@ -1060,15 +1091,20 @@ static void netlink_parse_fwd(struct netlink_parse_ctx *ctx,
|
||
|
reg1 = netlink_parse_register(nle, NFTNL_EXPR_FWD_SREG_DEV);
|
||
|
if (reg1) {
|
||
|
dev = netlink_get_register(ctx, loc, reg1);
|
||
|
- if (dev == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "fwd statement has no output expression");
|
||
|
+ if (dev == NULL) {
|
||
|
+ netlink_error(ctx, loc,
|
||
|
+ "fwd statement has no output expression");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
|
||
|
expr_set_type(dev, &ifindex_type, BYTEORDER_HOST_ENDIAN);
|
||
|
stmt->fwd.to = dev;
|
||
|
}
|
||
|
|
||
|
ctx->stmt = stmt;
|
||
|
+ return;
|
||
|
+out_err:
|
||
|
+ xfree(stmt);
|
||
|
}
|
||
|
|
||
|
static void netlink_parse_queue(struct netlink_parse_ctx *ctx,
|
||
|
@@ -1135,10 +1171,11 @@ static void netlink_parse_dynset(struct netlink_parse_ctx *ctx,
|
||
|
dnle = nftnl_expr_get(nle, NFTNL_EXPR_DYNSET_EXPR, NULL);
|
||
|
if (dnle != NULL) {
|
||
|
if (netlink_parse_expr(dnle, ctx) < 0)
|
||
|
- return;
|
||
|
- if (ctx->stmt == NULL)
|
||
|
- return netlink_error(ctx, loc,
|
||
|
- "Could not parse dynset stmt");
|
||
|
+ goto out_err;
|
||
|
+ if (ctx->stmt == NULL) {
|
||
|
+ netlink_error(ctx, loc, "Could not parse dynset stmt");
|
||
|
+ goto out_err;
|
||
|
+ }
|
||
|
dstmt = ctx->stmt;
|
||
|
}
|
||
|
|
||
|
@@ -1155,6 +1192,9 @@ static void netlink_parse_dynset(struct netlink_parse_ctx *ctx,
|
||
|
}
|
||
|
|
||
|
ctx->stmt = stmt;
|
||
|
+ return;
|
||
|
+out_err:
|
||
|
+ xfree(expr);
|
||
|
}
|
||
|
|
||
|
static void netlink_parse_objref(struct netlink_parse_ctx *ctx,
|
||
|
--
|
||
|
1.8.3.1
|
||
|
|