diff mbox series

[nft,1/2] parser_json: fix CMD_OBJ/NFT_OBJECT mismatch in delete path

Message ID 20260811134326.300827-2-palotasgergely@gmail.com
State New
Headers show
Series parser_json: fix JSON delete of ct stateful objects | expand

Commit Message

Gergely Palotas Aug. 11, 2026, 1:43 p.m. UTC 
The dispatch tables in json_parse_cmd_add() and json_parse_cmd_list()
pass NFT_OBJECT_CT_TIMEOUT, NFT_OBJECT_CT_EXPECT and NFT_OBJECT_TUNNEL
(kernel constants from <linux/netfilter/nf_tables.h>) as the cmd_obj
argument to json_parse_cmd_add_object(), which declares its parameter
as enum cmd_obj.

On the delete/list/destroy early-return path, cmd_obj is forwarded
directly to cmd_alloc(), which expects enum cmd_obj values. For ct
timeout, NFT_OBJECT_CT_TIMEOUT=7 aliases to CMD_OBJ_CHAIN=7, so a
JSON delete of a ct timeout is sent to the kernel as a delete chain
netlink message, returning EINVAL. The same mismatch affects ct
expectation and tunnel objects.

The add path was unaffected because the switch/case blocks inside the
function also used NFT_OBJECT_* constants and contained explicit
cmd_obj = CMD_OBJ_* assignments before falling through to cmd_alloc().
Those assignments are never reached on the delete path due to the
early return.

Fix this by using CMD_OBJ_* constants consistently in the dispatch
tables, matching the declared type of the parameter. Update the
switch cases and the CT_HELPER identity checks in the function
prologue to use CMD_OBJ_* as well, and drop the now-unnecessary
cmd_obj reassignments inside each case.

Signed-off-by: Gergely Palotas <palotasgergely@gmail.com>
---
 src/parser_json.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

Comments

Pablo Neira Ayuso Aug. 20, 2026, 11:48 a.m. UTC | #1 
On Tue, Aug 11, 2026 at 03:43:25PM +0200, Gergely Palotas wrote:
> The dispatch tables in json_parse_cmd_add() and json_parse_cmd_list()
> pass NFT_OBJECT_CT_TIMEOUT, NFT_OBJECT_CT_EXPECT and NFT_OBJECT_TUNNEL
> (kernel constants from <linux/netfilter/nf_tables.h>) as the cmd_obj
> argument to json_parse_cmd_add_object(), which declares its parameter
> as enum cmd_obj.
> 
> On the delete/list/destroy early-return path, cmd_obj is forwarded
> directly to cmd_alloc(), which expects enum cmd_obj values. For ct
> timeout, NFT_OBJECT_CT_TIMEOUT=7 aliases to CMD_OBJ_CHAIN=7, so a
> JSON delete of a ct timeout is sent to the kernel as a delete chain
> netlink message, returning EINVAL. The same mismatch affects ct
> expectation and tunnel objects.
> 
> The add path was unaffected because the switch/case blocks inside the
> function also used NFT_OBJECT_* constants and contained explicit
> cmd_obj = CMD_OBJ_* assignments before falling through to cmd_alloc().
> Those assignments are never reached on the delete path due to the
> early return.
> 
> Fix this by using CMD_OBJ_* constants consistently in the dispatch
> tables, matching the declared type of the parameter. Update the
> switch cases and the CT_HELPER identity checks in the function
> prologue to use CMD_OBJ_* as well, and drop the now-unnecessary
> cmd_obj reassignments inside each case.

cmd_alloc_obj_ct() expects NFT_OBJECT_CT_*

        switch (type) {
        case NFT_OBJECT_CT_HELPER:
                cmd_obj = CMD_OBJ_CT_HELPER;
                break;
        case NFT_OBJECT_CT_TIMEOUT:
                cmd_obj = CMD_OBJ_CT_TIMEOUT;
                break;
        case NFT_OBJECT_CT_EXPECT:
                cmd_obj = CMD_OBJ_CT_EXPECT;
                break;
        default:
                BUG("missing type mapping");
        }

Maybe this needs to be updated so this looks consistent?
diff mbox series

Patch

diff --git a/src/parser_json.c b/src/parser_json.c
index e0144b9b..6f40f8d8 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3792,11 +3792,11 @@  static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 			    "table", &h.table.name))
 		return NULL;
 	if ((op != CMD_DELETE ||
-	     cmd_obj == NFT_OBJECT_CT_HELPER) &&
+	     cmd_obj == CMD_OBJ_CT_HELPER) &&
 	    json_unpack_err(ctx, root, "{s:s}", "name", &h.obj.name)) {
 		return NULL;
 	} else if ((op == CMD_DELETE || op == CMD_DESTROY) &&
-		   cmd_obj != NFT_OBJECT_CT_HELPER &&
+		   cmd_obj != CMD_OBJ_CT_HELPER &&
 		   json_unpack(root, "{s:s}", "name", &h.obj.name) &&
 		   json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
 		json_error(ctx, "Either name or handle required to delete an object.");
@@ -3812,7 +3812,7 @@  static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 		h.obj.name = xstrdup(h.obj.name);
 
 	if (op == CMD_DELETE || op == CMD_LIST || op == CMD_DESTROY) {
-		if (cmd_obj == NFT_OBJECT_CT_HELPER)
+		if (cmd_obj == CMD_OBJ_CT_HELPER)
 			return cmd_alloc_obj_ct(op, NFT_OBJECT_CT_HELPER,
 						&h, int_loc, obj_alloc(int_loc));
 		return cmd_alloc(op, cmd_obj, &h, int_loc, NULL);
@@ -3849,8 +3849,7 @@  static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 			}
 		}
 		break;
-	case NFT_OBJECT_CT_HELPER:
-		cmd_obj = CMD_OBJ_CT_HELPER;
+	case CMD_OBJ_CT_HELPER:
 		obj->type = NFT_OBJECT_CT_HELPER;
 		if (!json_unpack(root, "{s:s}", "type", &tmp)) {
 			int ret;
@@ -3881,8 +3880,7 @@  static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 		}
 		obj->ct_helper.l3proto = l3proto;
 		break;
-	case NFT_OBJECT_CT_TIMEOUT:
-		cmd_obj = CMD_OBJ_CT_TIMEOUT;
+	case CMD_OBJ_CT_TIMEOUT:
 		init_list_head(&obj->ct_timeout.timeout_list);
 		obj->type = NFT_OBJECT_CT_TIMEOUT;
 		if (!json_unpack(root, "{s:s}", "protocol", &tmp)) {
@@ -3905,8 +3903,7 @@  static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 		if (json_parse_ct_timeout_policy(ctx, root, obj))
 			goto err_free_obj;
 		break;
-	case NFT_OBJECT_CT_EXPECT:
-		cmd_obj = CMD_OBJ_CT_EXPECT;
+	case CMD_OBJ_CT_EXPECT:
 		obj->type = NFT_OBJECT_CT_EXPECT;
 		if (!json_unpack(root, "{s:s}", "l3proto", &tmp) &&
 		    parse_family(tmp, &l3proto)) {
@@ -3972,8 +3969,7 @@  static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 
 		obj->synproxy.flags |= flags;
 		break;
-	case NFT_OBJECT_TUNNEL:
-		cmd_obj = CMD_OBJ_TUNNEL;
+	case CMD_OBJ_TUNNEL:
 		obj->type = NFT_OBJECT_TUNNEL;
 		if (json_parse_tunnel(ctx, root, obj))
 			goto err_free_obj;
@@ -4011,10 +4007,10 @@  static struct cmd *json_parse_cmd_add(struct json_ctx *ctx,
 		{ "flowtable", CMD_OBJ_FLOWTABLE, json_parse_cmd_add_flowtable },
 		{ "counter", CMD_OBJ_COUNTER, json_parse_cmd_add_object },
 		{ "quota", CMD_OBJ_QUOTA, json_parse_cmd_add_object },
-		{ "ct helper", NFT_OBJECT_CT_HELPER, json_parse_cmd_add_object },
-		{ "ct timeout", NFT_OBJECT_CT_TIMEOUT, json_parse_cmd_add_object },
-		{ "ct expectation", NFT_OBJECT_CT_EXPECT, json_parse_cmd_add_object },
-		{ "tunnel", NFT_OBJECT_TUNNEL, json_parse_cmd_add_object },
+		{ "ct helper", CMD_OBJ_CT_HELPER, json_parse_cmd_add_object },
+		{ "ct timeout", CMD_OBJ_CT_TIMEOUT, json_parse_cmd_add_object },
+		{ "ct expectation", CMD_OBJ_CT_EXPECT, json_parse_cmd_add_object },
+		{ "tunnel", CMD_OBJ_TUNNEL, json_parse_cmd_add_object },
 		{ "limit", CMD_OBJ_LIMIT, json_parse_cmd_add_object },
 		{ "secmark", CMD_OBJ_SECMARK, json_parse_cmd_add_object },
 		{ "synproxy", CMD_OBJ_SYNPROXY, json_parse_cmd_add_object }
@@ -4186,11 +4182,11 @@  static struct cmd *json_parse_cmd_list(struct json_ctx *ctx,
 		{ "counters", CMD_OBJ_COUNTERS, json_parse_cmd_list_multiple },
 		{ "quota", CMD_OBJ_QUOTA, json_parse_cmd_add_object },
 		{ "quotas", CMD_OBJ_QUOTAS, json_parse_cmd_list_multiple },
-		{ "ct helper", NFT_OBJECT_CT_HELPER, json_parse_cmd_add_object },
+		{ "ct helper", CMD_OBJ_CT_HELPER, json_parse_cmd_add_object },
 		{ "ct helpers", CMD_OBJ_CT_HELPERS, json_parse_cmd_list_multiple },
-		{ "ct timeout", NFT_OBJECT_CT_TIMEOUT, json_parse_cmd_add_object },
-		{ "ct expectation", NFT_OBJECT_CT_EXPECT, json_parse_cmd_add_object },
-		{ "tunnel", NFT_OBJECT_TUNNEL, json_parse_cmd_add_object },
+		{ "ct timeout", CMD_OBJ_CT_TIMEOUT, json_parse_cmd_add_object },
+		{ "ct expectation", CMD_OBJ_CT_EXPECT, json_parse_cmd_add_object },
+		{ "tunnel", CMD_OBJ_TUNNEL, json_parse_cmd_add_object },
 		{ "tunnels", CMD_OBJ_TUNNELS, json_parse_cmd_list_multiple },
 		{ "limit", CMD_OBJ_LIMIT, json_parse_cmd_add_object },
 		{ "limits", CMD_OBJ_LIMIT, json_parse_cmd_list_multiple },