All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nft] parser_json: initialize geneve options list for empty tunnel array
@ 2026-07-08 23:35 Omkhar Arasaratnam
  2026-07-09  8:05 ` Florian Westphal
  0 siblings, 1 reply; 2+ messages in thread
From: Omkhar Arasaratnam @ 2026-07-08 23:35 UTC (permalink / raw)
  To: netfilter-devel@vger.kernel.org
  Cc: pablo@netfilter.org, fw@strlen.de, fmancera@suse.de


[-- Attachment #1.1: Type: text/plain, Size: 966 bytes --]

Hi,

The attached patch fixes a crash in the JSON parser. A geneve tunnel object with an empty options array ("tunnel": []) leaves obj->tunnel.geneve_opts uninitialized; obj_tunnel_add_opts() then walks the uninitialized list head and nft -j crashes (SEGV in nftnl_tunnel_opt_geneve_set via near-NULL deref).

The equivalent native-syntax empty definition was already handled in f9047c1f ("evaluate: tunnel: don't assume src is set"); this covers the JSON parser path, which that fix did not reach. It is independent of Phil Sutter's pending "parser_json: Introduce json_parse_tunnel()" refactor, which relocates this block but carries the "if (index == 0)" guard forward unchanged -- happy to rebase onto or fold into that series if you prefer.

I'm sending the patch as an attachment because my mail client mangles inline patches; it applies cleanly with git am. The full commit message, crash trace, reproducer, and diff are in the attachment.

— oa



[-- Attachment #1.2: Type: text/html, Size: 2170 bytes --]

[-- Attachment #2: nftables-parse-0001.patch --]
[-- Type: application/octet-stream, Size: 3136 bytes --]

From de817ec180a37cc9af570809b632df984374fba4 Mon Sep 17 00:00:00 2001
From: Omkhar Arasaratnam <omkhar@linkedin.com>
Date: Wed, 8 Jul 2026 23:09:52 +0000
Subject: [PATCH nft] parser_json: initialize geneve options list for empty
 tunnel array

json_parse_cmd_add_object() only initializes obj->tunnel.geneve_opts on
the first iteration of the json_array_foreach() loop, guarded by
"if (index == 0)". A geneve tunnel object whose options array is empty
("tunnel": []) never enters the loop, so the list head is left
uninitialized. obj_tunnel_add_opts() (src/mnl.c) later walks it with
list_for_each_entry() and passes the bogus element to libnftnl, which
dereferences the near-NULL pointer and crashes nft:

  # nft -j -f empty_geneve.json
  AddressSanitizer: SEGV on unknown address 0x000000000012
    #1 nftnl_tunnel_opt_geneve_set  obj/tunnel.c:880
    #2 nftnl_tunnel_opt_set         obj/tunnel.c:910
    #3 obj_tunnel_add_opts          src/mnl.c:1608
    #4 mnl_nft_obj_add              src/mnl.c:1757
    #5 do_command_add               src/rule.c:1542
    #6 do_command                   src/rule.c:2802
    #10 main                        src/main.c:539

where empty_geneve.json is:

  { "nftables": [
    { "add": { "table": { "family": "netdev", "name": "x" } } },
    { "add": { "tunnel": { "family": "netdev", "name": "t", "table": "x",
        "src-ipv4": "192.168.2.10", "dst-ipv4": "192.168.2.11",
        "type": "geneve", "tunnel": [] } } } ] }

Initialize the list head unconditionally before the loop and drop the
per-iteration guard, so an empty array leaves a valid empty list. The
equivalent native-syntax empty definition was already handled in commit
f9047c1f ("evaluate: tunnel: don't assume src is set"); this covers the
JSON parser path, which that fix did not reach.

Fixes: 3a957f8f1ff1 ("tunnel: add tunnel object and statement json support")
Signed-off-by: Omkhar Arasaratnam <omkhar@linkedin.com>
---
Note: independent of Phil Sutter's pending series "Eliminate variable
declarations in switch cases" (nft PATCH 0/6, 2026-06-03), whose 2/6
"parser_json: Introduce json_parse_tunnel()" relocates this block but carries
the "if (index == 0)" guard forward unchanged. This is a standalone,
backportable crash fix; happy to rebase on top of that series or fold it in.
 src/parser_json.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index f04772a0..6a0c1745 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3941,6 +3941,8 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 					    "{s:o}", "tunnel", &tmp_json))
 				goto err_free_obj;
 
+			init_list_head(&obj->tunnel.geneve_opts);
+
 			json_array_foreach(tmp_json, index, value) {
 				struct tunnel_geneve *geneve = xmalloc(sizeof(struct tunnel_geneve));
 				if (!geneve)
@@ -3963,9 +3965,6 @@ static struct cmd *json_parse_cmd_add_object(struct json_ctx *ctx,
 					goto err_free_obj;
 				}
 
-				if (index == 0)
-					init_list_head(&obj->tunnel.geneve_opts);
-
 				list_add_tail(&geneve->list, &obj->tunnel.geneve_opts);
 			}
 			break;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH nft] parser_json: initialize geneve options list for empty tunnel array
  2026-07-08 23:35 [PATCH nft] parser_json: initialize geneve options list for empty tunnel array Omkhar Arasaratnam
@ 2026-07-09  8:05 ` Florian Westphal
  0 siblings, 0 replies; 2+ messages in thread
From: Florian Westphal @ 2026-07-09  8:05 UTC (permalink / raw)
  To: Omkhar Arasaratnam
  Cc: netfilter-devel@vger.kernel.org, pablo@netfilter.org,
	fmancera@suse.de

Omkhar Arasaratnam <omkhar@linkedin.com> wrote:
> The attached patch fixes a crash in the JSON parser. A geneve tunnel object with an empty options array ("tunnel": []) leaves obj->tunnel.geneve_opts uninitialized; obj_tunnel_add_opts() then walks the uninitialized list head and nft -j crashes (SEGV in nftnl_tunnel_opt_geneve_set via near-NULL deref).

Applied.  I moved the crasher to testcases/bogons.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-09  8:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 23:35 [PATCH nft] parser_json: initialize geneve options list for empty tunnel array Omkhar Arasaratnam
2026-07-09  8:05 ` Florian Westphal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.