[PATCH nft 0/5] assorted updates and fixes

[PATCH nft 0/5] assorted updates and fixes

[Pablo Neira Ayuso]

Hi,

This batch contains several assorted updates and fixes:

1) Skip lookup for mistyped names if handle is used.
2) Assert of non-nul name when {table,chain,obj,flowtable}_cache_find()
   is used to catch for bugs when handle is used.
3) Consolidate repetitive cache name hash.
4) Restrict reset command to use name only because NFT_MSG_GETSET and
   NFT_MSG_GETSETELEM is missing lookup by handle in the kernel.
5) Allow to delete a map with handle, for consistency with the existing
   command to delete a set.

Pablo Neira Ayuso (5):
  rule: skip fuzzy lookup if object name is not available
  cache: assert name is non-nul when looking up
  cache: pass name to cache_add()
  parser_bison: only reset by name is supported by now
  parser_bison: allow delete command with map via handle

 include/cache.h                               |  2 +-
 src/cache.c                                   | 60 ++++++++-----------
 src/parser_bison.y                            |  6 +-
 src/rule.c                                    | 12 ++++
 .../bogons/nft-f/null_set_name_crash          |  2 +
 .../testcases/cache/0008_delete_by_handle_0   |  4 ++
 .../cache/0009_delete_by_handle_incorrect_0   |  1 +
 7 files changed, 47 insertions(+), 40 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/null_set_name_crash

-- 
2.30.2

[PATCH nft 1/5] rule: skip fuzzy lookup if object name is not available

[Pablo Neira Ayuso]

Skip fuzzy lookup for suggestions when handles are used.

Note that 4cf97abfee61 ("rule: Avoid segfault with anonymous chains")
already skips it for chain.

Fixes: 285bb67a11ad ("src: introduce simple hints on incorrect set")
Fixes: 9f7817a4e022 ("src: introduce simple hints on incorrect chain")
Fixes: d7476ddd5f7d ("src: introduce simple hints on incorrect table")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/rule.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/rule.c b/src/rule.c
index 80315837baf0..a5a668f9992c 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -211,6 +211,9 @@ struct set *set_lookup_fuzzy(const char *set_name,
 	struct table *table;
 	struct set *set;
 
+	if (!set_name)
+		return NULL;
+
 	string_misspell_init(&st);
 
 	list_for_each_entry(table, &cache->table_cache.list, cache.list) {
@@ -1214,6 +1217,9 @@ struct table *table_lookup_fuzzy(const struct handle *h,
 	struct string_misspell_state st;
 	struct table *table;
 
+	if (!h->table.name)
+		return NULL;
+
 	string_misspell_init(&st);
 
 	list_for_each_entry(table, &cache->table_cache.list, cache.list) {
@@ -1696,6 +1702,9 @@ struct obj *obj_lookup_fuzzy(const char *obj_name,
 	struct table *table;
 	struct obj *obj;
 
+	if (!obj_name)
+		return NULL;
+
 	string_misspell_init(&st);
 
 	list_for_each_entry(table, &cache->table_cache.list, cache.list) {
@@ -2191,6 +2200,9 @@ struct flowtable *flowtable_lookup_fuzzy(const char *ft_name,
 	struct table *table;
 	struct flowtable *ft;
 
+	if (!ft_name)
+		return NULL;
+
 	string_misspell_init(&st);
 
 	list_for_each_entry(table, &cache->table_cache.list, cache.list) {
-- 
2.30.2

[PATCH nft 2/5] cache: assert name is non-nul when looking up

[Pablo Neira Ayuso]

{table,chain,set,obj,flowtable}_cache_find() should not be called when
handles are used

Fixes: 5ec5c706d993 ("cache: add hashtable cache for table")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/cache.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 3ac819cf68fb..d16052c608d5 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -551,8 +551,7 @@ struct table *table_cache_find(const struct cache *cache,
 	struct table *table;
 	uint32_t hash;
 
-	if (!name)
-		return NULL;
+	assert(name);
 
 	hash = djb_hash(name) % NFT_CACHE_HSIZE;
 	list_for_each_entry(table, &cache->ht[hash], cache.hlist) {
@@ -672,6 +671,8 @@ struct chain *chain_cache_find(const struct table *table, const char *name)
 	struct chain *chain;
 	uint32_t hash;
 
+	assert(name);
+
 	hash = djb_hash(name) % NFT_CACHE_HSIZE;
 	list_for_each_entry(chain, &table->chain_cache.ht[hash], cache.hlist) {
 		if (!strcmp(chain->handle.chain.name, name))
@@ -840,6 +841,8 @@ struct set *set_cache_find(const struct table *table, const char *name)
 	struct set *set;
 	uint32_t hash;
 
+	assert(name);
+
 	hash = djb_hash(name) % NFT_CACHE_HSIZE;
 	list_for_each_entry(set, &table->set_cache.ht[hash], cache.hlist) {
 		if (!strcmp(set->handle.set.name, name))
@@ -954,6 +957,8 @@ struct obj *obj_cache_find(const struct table *table, const char *name,
 	struct obj *obj;
 	uint32_t hash;
 
+	assert(name);
+
 	hash = djb_hash(name) % NFT_CACHE_HSIZE;
 	list_for_each_entry(obj, &table->obj_cache.ht[hash], cache.hlist) {
 		if (!strcmp(obj->handle.obj.name, name) &&
@@ -1058,6 +1063,8 @@ struct flowtable *ft_cache_find(const struct table *table, const char *name)
 	struct flowtable *ft;
 	uint32_t hash;
 
+	assert(name);
+
 	hash = djb_hash(name) % NFT_CACHE_HSIZE;
 	list_for_each_entry(ft, &table->ft_cache.ht[hash], cache.hlist) {
 		if (!strcmp(ft->handle.flowtable.name, name))
-- 
2.30.2

[PATCH nft 3/5] cache: pass name to cache_add()

[Pablo Neira Ayuso]

Consolidate the name hash in the cache_add() function.

No functional changes are intended.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/cache.h |  2 +-
 src/cache.c     | 49 +++++++++++++++----------------------------------
 2 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/include/cache.h b/include/cache.h
index e6bde6c6bee3..c969534f4ea7 100644
--- a/include/cache.h
+++ b/include/cache.h
@@ -123,7 +123,7 @@ struct cache_item {
 
 void cache_init(struct cache *cache);
 void cache_free(struct cache *cache);
-void cache_add(struct cache_item *item, struct cache *cache, uint32_t hash);
+void cache_add(struct cache_item *item, struct cache *cache, const char *name);
 void cache_del(struct cache_item *item);
 
 void table_cache_add(struct table *table, struct nft_cache *cache);
diff --git a/src/cache.c b/src/cache.c
index d16052c608d5..d58fb59ff061 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -534,10 +534,7 @@ int nft_cache_evaluate(struct nft_ctx *nft, struct list_head *cmds,
 
 void table_cache_add(struct table *table, struct nft_cache *cache)
 {
-	uint32_t hash;
-
-	hash = djb_hash(table->handle.table.name) % NFT_CACHE_HSIZE;
-	cache_add(&table->cache, &cache->table_cache, hash);
+	cache_add(&table->cache, &cache->table_cache, table->handle.table.name);
 }
 
 void table_cache_del(struct table *table)
@@ -572,8 +569,8 @@ static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
 {
 	struct chain_cache_dump_ctx *ctx = arg;
 	const char *chain_name, *table_name;
-	uint32_t hash, family;
 	struct chain *chain;
+	uint32_t family;
 
 	table_name = nftnl_chain_get_str(nlc, NFTNL_CHAIN_TABLE);
 	family = nftnl_chain_get_u32(nlc, NFTNL_CHAIN_FAMILY);
@@ -583,13 +580,12 @@ static int chain_cache_cb(struct nftnl_chain *nlc, void *arg)
 		return 0;
 
 	chain_name = nftnl_chain_get_str(nlc, NFTNL_CHAIN_NAME);
-	hash = djb_hash(chain_name) % NFT_CACHE_HSIZE;
-	chain = netlink_delinearize_chain(ctx->nlctx, nlc);
 
+	chain = netlink_delinearize_chain(ctx->nlctx, nlc);
 	if (chain->flags & CHAIN_F_BINDING) {
 		list_add_tail(&chain->cache.list, &ctx->table->chain_bindings);
 	} else {
-		cache_add(&chain->cache, &ctx->table->chain_cache, hash);
+		cache_add(&chain->cache, &ctx->table->chain_cache, chain_name);
 	}
 
 	nftnl_chain_list_del(nlc);
@@ -655,10 +651,7 @@ void nft_chain_cache_update(struct netlink_ctx *ctx, struct table *table,
 
 void chain_cache_add(struct chain *chain, struct table *table)
 {
-	uint32_t hash;
-
-	hash = djb_hash(chain->handle.chain.name) % NFT_CACHE_HSIZE;
-	cache_add(&chain->cache, &table->chain_cache, hash);
+	cache_add(&chain->cache, &table->chain_cache, chain->handle.chain.name);
 }
 
 void chain_cache_del(struct chain *chain)
@@ -760,7 +753,6 @@ static int set_cache_cb(struct nftnl_set *nls, void *arg)
 	const char *set_name;
 	uint32_t set_family;
 	struct set *set;
-	uint32_t hash;
 
 	set_table = nftnl_set_get_str(nls, NFTNL_SET_TABLE);
 	set_family = nftnl_set_get_u32(nls, NFTNL_SET_FAMILY);
@@ -774,8 +766,7 @@ static int set_cache_cb(struct nftnl_set *nls, void *arg)
 		return -1;
 
 	set_name = nftnl_set_get_str(nls, NFTNL_SET_NAME);
-	hash = djb_hash(set_name) % NFT_CACHE_HSIZE;
-	cache_add(&set->cache, &ctx->table->set_cache, hash);
+	cache_add(&set->cache, &ctx->table->set_cache, set_name);
 
 	nftnl_set_list_del(nls);
 	nftnl_set_free(nls);
@@ -825,10 +816,7 @@ set_cache_dump(struct netlink_ctx *ctx,
 
 void set_cache_add(struct set *set, struct table *table)
 {
-	uint32_t hash;
-
-	hash = djb_hash(set->handle.set.name) % NFT_CACHE_HSIZE;
-	cache_add(&set->cache, &table->set_cache, hash);
+	cache_add(&set->cache, &table->set_cache, set->handle.set.name);
 }
 
 void set_cache_del(struct set *set)
@@ -864,7 +852,6 @@ static int obj_cache_cb(struct nftnl_obj *nlo, void *arg)
 	const char *obj_name;
 	uint32_t obj_family;
 	struct obj *obj;
-	uint32_t hash;
 
 	obj_table = nftnl_obj_get_str(nlo, NFTNL_OBJ_TABLE);
 	obj_family = nftnl_obj_get_u32(nlo, NFTNL_OBJ_FAMILY);
@@ -876,8 +863,7 @@ static int obj_cache_cb(struct nftnl_obj *nlo, void *arg)
 	obj = netlink_delinearize_obj(ctx->nlctx, nlo);
 	if (obj) {
 		obj_name = nftnl_obj_get_str(nlo, NFTNL_OBJ_NAME);
-		hash = djb_hash(obj_name) % NFT_CACHE_HSIZE;
-		cache_add(&obj->cache, &ctx->table->obj_cache, hash);
+		cache_add(&obj->cache, &ctx->table->obj_cache, obj_name);
 	}
 
 	nftnl_obj_list_del(nlo);
@@ -940,10 +926,7 @@ static struct nftnl_obj_list *obj_cache_dump(struct netlink_ctx *ctx,
 
 void obj_cache_add(struct obj *obj, struct table *table)
 {
-	uint32_t hash;
-
-	hash = djb_hash(obj->handle.obj.name) % NFT_CACHE_HSIZE;
-	cache_add(&obj->cache, &table->obj_cache, hash);
+	cache_add(&obj->cache, &table->obj_cache, obj->handle.obj.name);
 }
 
 void obj_cache_del(struct obj *obj)
@@ -981,7 +964,6 @@ static int ft_cache_cb(struct nftnl_flowtable *nlf, void *arg)
 	const char *ft_table;
 	const char *ft_name;
 	uint32_t ft_family;
-	uint32_t hash;
 
 	ft_family = nftnl_flowtable_get_u32(nlf, NFTNL_FLOWTABLE_FAMILY);
 	ft_table = nftnl_flowtable_get_str(nlf, NFTNL_FLOWTABLE_TABLE);
@@ -995,8 +977,7 @@ static int ft_cache_cb(struct nftnl_flowtable *nlf, void *arg)
 		return -1;
 
 	ft_name = nftnl_flowtable_get_str(nlf, NFTNL_FLOWTABLE_NAME);
-	hash = djb_hash(ft_name) % NFT_CACHE_HSIZE;
-	cache_add(&ft->cache, &ctx->table->ft_cache, hash);
+	cache_add(&ft->cache, &ctx->table->ft_cache, ft_name);
 
 	nftnl_flowtable_list_del(nlf);
 	nftnl_flowtable_free(nlf);
@@ -1047,10 +1028,7 @@ ft_cache_dump(struct netlink_ctx *ctx, const struct nft_cache_filter *filter)
 
 void ft_cache_add(struct flowtable *ft, struct table *table)
 {
-	uint32_t hash;
-
-	hash = djb_hash(ft->handle.flowtable.name) % NFT_CACHE_HSIZE;
-	cache_add(&ft->cache, &table->ft_cache, hash);
+	cache_add(&ft->cache, &table->ft_cache, ft->handle.flowtable.name);
 }
 
 void ft_cache_del(struct flowtable *ft)
@@ -1389,8 +1367,11 @@ void cache_free(struct cache *cache)
 	free(cache->ht);
 }
 
-void cache_add(struct cache_item *item, struct cache *cache, uint32_t hash)
+void cache_add(struct cache_item *item, struct cache *cache, const char *name)
 {
+	uint32_t hash;
+
+	hash = djb_hash(name) % NFT_CACHE_HSIZE;
 	list_add_tail(&item->hlist, &cache->ht[hash]);
 	list_add_tail(&item->list, &cache->list);
 }
-- 
2.30.2

[PATCH nft 4/5] parser_bison: only reset by name is supported by now

[Pablo Neira Ayuso]

NFT_MSG_GETSET does not support for handle lookup yet, restrict this to
reset by name by now.

Add a bogon test reported by Florian Westphal.

Fixes: 83e0f4402fb7 ("Implement 'reset {set,map,element}' commands")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/parser_bison.y                                     | 4 ++--
 tests/shell/testcases/bogons/nft-f/null_set_name_crash | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/null_set_name_crash

diff --git a/src/parser_bison.y b/src/parser_bison.y
index ed6a24a15377..87b34293d22c 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1757,11 +1757,11 @@ reset_cmd		:	COUNTERS	list_cmd_spec_any
 			{
 				$$ = cmd_alloc(CMD_RESET, CMD_OBJ_ELEMENTS, &$2, &@$, $3);
 			}
-			|	SET		set_or_id_spec
+			|	SET		set_spec
 			{
 				$$ = cmd_alloc(CMD_RESET, CMD_OBJ_SET, &$2, &@$, NULL);
 			}
-			|	MAP		set_or_id_spec
+			|	MAP		set_spec
 			{
 				$$ = cmd_alloc(CMD_RESET, CMD_OBJ_MAP, &$2, &@$, NULL);
 			}
diff --git a/tests/shell/testcases/bogons/nft-f/null_set_name_crash b/tests/shell/testcases/bogons/nft-f/null_set_name_crash
new file mode 100644
index 000000000000..e5d85b228a84
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/null_set_name_crash
@@ -0,0 +1,2 @@
+table y { }
+reset set y handle 6
-- 
2.30.2

[PATCH nft 5/5] parser_bison: allow delete command with map via handle

[Pablo Neira Ayuso]

For consistency with sets, allow delete via handle for maps too.

This fix requires the handle hashtable lookup infrastructure.

Fixes: f4a34d25f6d5 ("src: list set handle and delete set via set handle")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/parser_bison.y                                            | 2 +-
 tests/shell/testcases/cache/0008_delete_by_handle_0           | 4 ++++
 tests/shell/testcases/cache/0009_delete_by_handle_incorrect_0 | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 87b34293d22c..9278b67a2931 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -1450,7 +1450,7 @@ delete_cmd		:	TABLE		table_or_id_spec
 			{
 				$$ = cmd_alloc(CMD_DELETE, CMD_OBJ_SET, &$2, &@$, NULL);
 			}
-			|	MAP		set_spec
+			|	MAP		set_or_id_spec
 			{
 				$$ = cmd_alloc(CMD_DELETE, CMD_OBJ_SET, &$2, &@$, NULL);
 			}
diff --git a/tests/shell/testcases/cache/0008_delete_by_handle_0 b/tests/shell/testcases/cache/0008_delete_by_handle_0
index 0db4c693f6d4..9eb75e6ce374 100755
--- a/tests/shell/testcases/cache/0008_delete_by_handle_0
+++ b/tests/shell/testcases/cache/0008_delete_by_handle_0
@@ -16,6 +16,10 @@ $NFT add set t s { type ipv4_addr\; }
 HANDLE=`$NFT -a list ruleset | grep "set.*handle" | cut -d' ' -f6`
 $NFT delete set t handle $HANDLE
 
+$NFT add map t m { type ipv4_addr : ipv4_addr\; }
+HANDLE=`$NFT -a list ruleset | grep "map.*handle" | cut -d' ' -f6`
+$NFT delete map t handle $HANDLE
+
 $NFT add flowtable t f { hook ingress priority 0\; devices = { lo } \; }
 HANDLE=`$NFT -a list ruleset | grep "flowtable.*handle" | cut -d' ' -f6`
 $NFT delete flowtable t handle $HANDLE
diff --git a/tests/shell/testcases/cache/0009_delete_by_handle_incorrect_0 b/tests/shell/testcases/cache/0009_delete_by_handle_incorrect_0
index f0bb02a636ee..dd390e73c8e8 100755
--- a/tests/shell/testcases/cache/0009_delete_by_handle_incorrect_0
+++ b/tests/shell/testcases/cache/0009_delete_by_handle_incorrect_0
@@ -3,6 +3,7 @@
 $NFT delete table handle 4000 && exit 1
 $NFT delete chain t handle 4000 && exit 1
 $NFT delete set t handle 4000 && exit 1
+$NFT delete map t handle 4000 && exit 1
 $NFT delete flowtable t handle 4000 && exit 1
 $NFT delete counter t handle 4000 && exit 1
 exit 0
-- 
2.30.2

Re: [PATCH nft 0/5] assorted updates and fixes

[Pablo Neira Ayuso]

On Sun, Jun 15, 2025 at 12:00:14PM +0200, Pablo Neira Ayuso wrote:
> Hi,
> 
> This batch contains several assorted updates and fixes:
> 
> 1) Skip lookup for mistyped names if handle is used.
> 2) Assert of non-nul name when {table,chain,obj,flowtable}_cache_find()
>    is used to catch for bugs when handle is used.
> 3) Consolidate repetitive cache name hash.
> 4) Restrict reset command to use name only because NFT_MSG_GETSET and
>    NFT_MSG_GETSETELEM is missing lookup by handle in the kernel.
> 5) Allow to delete a map with handle, for consistency with the existing
>    command to delete a set.

Pushed out.