From 5e9e2dc7e972f6bbbc0156ad97b4ee9d11fcb837 Mon Sep 17 00:00:00 2001 From: Phil Sutter 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 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 Signed-off-by: Pablo Neira Ayuso --- 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