Each object (secmark, synproxy, quota, limit, counter) is dynamically allocated
by the parser and not freed at exit.
However, there is no need to use dynamic allocation here because struct obj
already provides the required storage. Update the grammar to ensure that
obj_alloc() is called before config occurs.
This fixes the following memleak (secmark as example):
# valgrind --leak-check=full nft add secmark inet raw ssh \"system_u:object_r:ssh_server_packet_t:s0\"
==14643== Memcheck, a memory error detector
==14643== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==14643== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==14643== Command: nft add secmark inet raw ssh "system_u:object_r:ssh_server_packet_t:s0"
==14643==
==14643==
==14643== HEAP SUMMARY:
==14643== in use at exit: 256 bytes in 1 blocks
==14643== total heap usage: 41 allocs, 40 frees, 207,809 bytes allocated
==14643==
==14643== 256 bytes in 1 blocks are definitely lost in loss record 1 of 1
==14643== at 0x4C29EA3: malloc (vg_replace_malloc.c:309)
==14643== by 0x4E72074: xmalloc (utils.c:36)
==14643== by 0x4E72074: xzalloc (utils.c:65)
==14643== by 0x4E89A31: nft_parse (parser_bison.y:3706)
==14643== by 0x4E778E7: nft_parse_bison_buffer (libnftables.c:375)
==14643== by 0x4E778E7: nft_run_cmd_from_buffer (libnftables.c:443)
==14643== by 0x40170F: main (main.c:326)
Fixes: f44ab88b1088e ("src: add synproxy stateful object support")
Fixes: 3bc84e5c1fdd1 ("src: add support for setting secmark")
Fixes: c0697eabe832d ("src: add stateful object support for limit")
Fixes: 4d38878b39be4 ("src: add/create/delete stateful objects")
Signed-off-by: Eric Jallot <ejallot@xxxxxxxxx>
---
src/parser_bison.y | 83 ++++++++++++++++++------------------------------------
1 file changed, 27 insertions(+), 56 deletions(-)
diff --git a/src/parser_bison.y b/src/parser_bison.y
index cd249c82d938..1e2b30015f78 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -149,12 +149,7 @@ int nft_lex(void *, void *, void *);
struct set *set;
struct obj *obj;
struct flowtable *flowtable;
- struct counter *counter;
- struct quota *quota;
- struct secmark *secmark;
- struct synproxy *synproxy;
struct ct *ct;
- struct limit *limit;
const struct datatype *datatype;
struct handle_spec handle_spec;
struct position_spec position_spec;
@@ -782,16 +777,6 @@ int nft_lex(void *, void *, void *);
%destructor { xfree($$); } monitor_event
%type <val> monitor_object monitor_format
-%type <counter> counter_config
-%destructor { xfree($$); } counter_config
-%type <quota> quota_config
-%destructor { xfree($$); } quota_config
-%type <limit> limit_config
-%destructor { xfree($$); } limit_config
-%type <secmark> secmark_config
-%destructor { xfree($$); } secmark_config
-%type <synproxy> synproxy_config
-%destructor { xfree($$); } synproxy_config
%type <val> synproxy_ts synproxy_sack
%type <expr> tcp_hdr_expr
@@ -989,17 +974,16 @@ add_cmd : TABLE table_spec
handle_merge(&obj->handle, &$2);
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, obj);
}
- | COUNTER obj_spec counter_obj
+ | COUNTER obj_spec counter_obj counter_config
{
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_COUNTER, &$2, &@$, $3);
}
- | QUOTA obj_spec quota_obj
+ | QUOTA obj_spec quota_obj quota_config
{
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_QUOTA, &$2, &@$, $3);
}
| CT HELPER obj_spec ct_obj_alloc '{' ct_helper_block '}'
{
-
$$ = cmd_alloc_obj_ct(CMD_ADD, NFT_OBJECT_CT_HELPER, &$3, &@$, $4);
}
| CT TIMEOUT obj_spec ct_obj_alloc '{' ct_timeout_block '}'
@@ -1010,15 +994,15 @@ add_cmd : TABLE table_spec
{
$$ = cmd_alloc_obj_ct(CMD_ADD, NFT_OBJECT_CT_EXPECT, &$3, &@$, $4);
}
- | LIMIT obj_spec limit_obj
+ | LIMIT obj_spec limit_obj limit_config
{
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_LIMIT, &$2, &@$, $3);
}
- | SECMARK obj_spec secmark_obj
+ | SECMARK obj_spec secmark_obj secmark_config
{
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_SECMARK, &$2, &@$, $3);
}
- | SYNPROXY obj_spec synproxy_obj
+ | SYNPROXY obj_spec synproxy_obj synproxy_config
{
$$ = cmd_alloc(CMD_ADD, CMD_OBJ_SYNPROXY, &$2, &@$, $3);
}
@@ -1087,11 +1071,11 @@ create_cmd : TABLE table_spec
handle_merge(&obj->handle, &$2);
$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_COUNTER, &$2, &@$, obj);
}
- | COUNTER obj_spec counter_obj
+ | COUNTER obj_spec counter_obj counter_config
{
$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_COUNTER, &$2, &@$, $3);
}
- | QUOTA obj_spec quota_obj
+ | QUOTA obj_spec quota_obj quota_config
{
$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_QUOTA, &$2, &@$, $3);
}
@@ -1107,15 +1091,15 @@ create_cmd : TABLE table_spec
{
$$ = cmd_alloc_obj_ct(CMD_CREATE, NFT_OBJECT_CT_EXPECT, &$3, &@$, $4);
}
- | LIMIT obj_spec limit_obj
+ | LIMIT obj_spec limit_obj limit_config
{
$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_LIMIT, &$2, &@$, $3);
}
- | SECMARK obj_spec secmark_obj
+ | SECMARK obj_spec secmark_obj secmark_config
{
$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_SECMARK, &$2, &@$, $3);
}
- | SYNPROXY obj_spec synproxy_obj
+ | SYNPROXY obj_spec synproxy_obj synproxy_config
{
$$ = cmd_alloc(CMD_CREATE, CMD_OBJ_SYNPROXY, &$2, &@$, $3);
}
@@ -1911,7 +1895,6 @@ counter_block : /* empty */ { $$ = $<obj>-1; }
| counter_block stmt_separator
| counter_block counter_config
{
- $1->counter = *$2;
$$ = $1;
}
;
@@ -1921,7 +1904,6 @@ quota_block : /* empty */ { $$ = $<obj>-1; }
| quota_block stmt_separator
| quota_block quota_config
{
- $1->quota = *$2;
$$ = $1;
}
;
@@ -1958,7 +1940,6 @@ limit_block : /* empty */ { $$ = $<obj>-1; }
| limit_block stmt_separator
| limit_block limit_config
{
- $1->limit = *$2;
$$ = $1;
}
;
@@ -1968,7 +1949,6 @@ secmark_block : /* empty */ { $$ = $<obj>-1; }
| secmark_block stmt_separator
| secmark_block secmark_config
{
- $1->secmark = *$2;
$$ = $1;
}
;
@@ -1978,7 +1958,6 @@ synproxy_block : /* empty */ { $$ = $<obj>-1; }
| synproxy_block stmt_separator
| synproxy_block synproxy_config
{
- $1->synproxy = *$2;
$$ = $1;
}
;
@@ -2883,7 +2862,7 @@ synproxy_config : MSS NUM WSCALE NUM synproxy_ts synproxy_sack
struct synproxy *synproxy;
uint32_t flags = 0;
- synproxy = xzalloc(sizeof(*synproxy));
+ synproxy = &$<obj>0->synproxy;
synproxy->mss = $2;
flags |= NF_SYNPROXY_OPT_MSS;
synproxy->wscale = $4;
@@ -2893,14 +2872,13 @@ synproxy_config : MSS NUM WSCALE NUM synproxy_ts synproxy_sack
if ($6)
flags |= $6;
synproxy->flags = flags;
- $$ = synproxy;
}
| MSS NUM stmt_separator WSCALE NUM stmt_separator synproxy_ts synproxy_sack
{
struct synproxy *synproxy;
uint32_t flags = 0;
- synproxy = xzalloc(sizeof(*synproxy));
+ synproxy = &$<obj>0->synproxy;
synproxy->mss = $2;
flags |= NF_SYNPROXY_OPT_MSS;
synproxy->wscale = $5;
@@ -2910,15 +2888,13 @@ synproxy_config : MSS NUM WSCALE NUM synproxy_ts synproxy_sack
if ($8)
flags |= $8;
synproxy->flags = flags;
- $$ = synproxy;
}
;
-synproxy_obj : synproxy_config
+synproxy_obj : /* empty */
{
$$ = obj_alloc(&@$);
$$->type = NFT_OBJECT_SYNPROXY;
- $$->synproxy = *$1;
}
;
@@ -3655,18 +3631,16 @@ counter_config : PACKETS NUM BYTES NUM
{
struct counter *counter;
- counter = xzalloc(sizeof(*counter));
+ counter = &$<obj>0->counter;
counter->packets = $2;
counter->bytes = $4;
- $$ = counter;
}
;
-counter_obj : counter_config
+counter_obj : /* empty */
{
$$ = obj_alloc(&@$);
$$->type = NFT_OBJECT_COUNTER;
- $$->counter = *$1;
}
;
@@ -3683,19 +3657,17 @@ quota_config : quota_mode NUM quota_unit quota_used
YYERROR;
}
- quota = xzalloc(sizeof(*quota));
+ quota = &$<obj>0->quota;
quota->bytes = $2 * rate;
quota->used = $4;
quota->flags = $1;
- $$ = quota;
}
;
-quota_obj : quota_config
+quota_obj : /* empty */
{
$$ = obj_alloc(&@$);
$$->type = NFT_OBJECT_QUOTA;
- $$->quota = *$1;
}
;
@@ -3703,21 +3675,22 @@ secmark_config : string
{
int ret;
struct secmark *secmark;
- secmark = xzalloc(sizeof(*secmark));
+
+ secmark = &$<obj>0->secmark;
ret = snprintf(secmark->ctx, sizeof(secmark->ctx), "%s", $1);
if (ret <= 0 || ret >= (int)sizeof(secmark->ctx)) {
erec_queue(error(&@1, "invalid context '%s', max length is %u\n", $1, (int)sizeof(secmark->ctx)), state->msgs);
+ xfree($1);
YYERROR;
}
- $$ = secmark;
+ xfree($1);
}
;
-secmark_obj : secmark_config
+secmark_obj : /* empty */
{
$$ = obj_alloc(&@$);
$$->type = NFT_OBJECT_SECMARK;
- $$->secmark = *$1;
}
;
@@ -3822,7 +3795,7 @@ ct_expect_config : PROTOCOL ct_l4protoname stmt_separator
}
;
-ct_obj_alloc :
+ct_obj_alloc : /* empty */
{
$$ = obj_alloc(&@$);
}
@@ -3831,13 +3804,13 @@ ct_obj_alloc :
limit_config : RATE limit_mode NUM SLASH time_unit limit_burst_pkts
{
struct limit *limit;
- limit = xzalloc(sizeof(*limit));
+
+ limit = &$<obj>0->limit;
limit->rate = $3;
limit->unit = $5;
limit->burst = $6;
limit->type = NFT_LIMIT_PKTS;
limit->flags = $2;
- $$ = limit;
}
| RATE limit_mode NUM STRING limit_burst_bytes
{
@@ -3851,21 +3824,19 @@ limit_config : RATE limit_mode NUM SLASH time_unit limit_burst_pkts
YYERROR;
}
- limit = xzalloc(sizeof(*limit));
+ limit = &$<obj>0->limit;
limit->rate = rate * $3;
limit->unit = unit;
limit->burst = $5;
limit->type = NFT_LIMIT_PKT_BYTES;
limit->flags = $2;
- $$ = limit;
}
;
-limit_obj : limit_config
+limit_obj : /* empty */
{
$$ = obj_alloc(&@$);
$$->type = NFT_OBJECT_LIMIT;
- $$->limit = *$1;
}
;
--
2.11.0
