* [PATCH nft 0/5] assorted updates and fixes
@ 2025-06-15 10:00 Pablo Neira Ayuso
2025-06-15 10:00 ` [PATCH nft 1/5] rule: skip fuzzy lookup if object name is not available Pablo Neira Ayuso
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: Pablo Neira Ayuso @ 2025-06-15 10:00 UTC (permalink / raw)
To: netfilter-devel
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH nft 1/5] rule: skip fuzzy lookup if object name is not available 2025-06-15 10:00 [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso @ 2025-06-15 10:00 ` Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 2/5] cache: assert name is non-nul when looking up Pablo Neira Ayuso ` (4 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2025-06-15 10:00 UTC (permalink / raw) To: netfilter-devel 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH nft 2/5] cache: assert name is non-nul when looking up 2025-06-15 10:00 [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 1/5] rule: skip fuzzy lookup if object name is not available Pablo Neira Ayuso @ 2025-06-15 10:00 ` Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 3/5] cache: pass name to cache_add() Pablo Neira Ayuso ` (3 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2025-06-15 10:00 UTC (permalink / raw) To: netfilter-devel {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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH nft 3/5] cache: pass name to cache_add() 2025-06-15 10:00 [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 1/5] rule: skip fuzzy lookup if object name is not available Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 2/5] cache: assert name is non-nul when looking up Pablo Neira Ayuso @ 2025-06-15 10:00 ` Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 4/5] parser_bison: only reset by name is supported by now Pablo Neira Ayuso ` (2 subsequent siblings) 5 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2025-06-15 10:00 UTC (permalink / raw) To: netfilter-devel 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH nft 4/5] parser_bison: only reset by name is supported by now 2025-06-15 10:00 [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso ` (2 preceding siblings ...) 2025-06-15 10:00 ` [PATCH nft 3/5] cache: pass name to cache_add() Pablo Neira Ayuso @ 2025-06-15 10:00 ` Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 5/5] parser_bison: allow delete command with map via handle Pablo Neira Ayuso 2025-06-23 17:08 ` [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso 5 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2025-06-15 10:00 UTC (permalink / raw) To: netfilter-devel 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH nft 5/5] parser_bison: allow delete command with map via handle 2025-06-15 10:00 [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso ` (3 preceding siblings ...) 2025-06-15 10:00 ` [PATCH nft 4/5] parser_bison: only reset by name is supported by now Pablo Neira Ayuso @ 2025-06-15 10:00 ` Pablo Neira Ayuso 2025-06-23 17:08 ` [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso 5 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2025-06-15 10:00 UTC (permalink / raw) To: netfilter-devel 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH nft 0/5] assorted updates and fixes 2025-06-15 10:00 [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso ` (4 preceding siblings ...) 2025-06-15 10:00 ` [PATCH nft 5/5] parser_bison: allow delete command with map via handle Pablo Neira Ayuso @ 2025-06-23 17:08 ` Pablo Neira Ayuso 5 siblings, 0 replies; 7+ messages in thread From: Pablo Neira Ayuso @ 2025-06-23 17:08 UTC (permalink / raw) To: netfilter-devel 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-06-23 17:08 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-15 10:00 [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 1/5] rule: skip fuzzy lookup if object name is not available Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 2/5] cache: assert name is non-nul when looking up Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 3/5] cache: pass name to cache_add() Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 4/5] parser_bison: only reset by name is supported by now Pablo Neira Ayuso 2025-06-15 10:00 ` [PATCH nft 5/5] parser_bison: allow delete command with map via handle Pablo Neira Ayuso 2025-06-23 17:08 ` [PATCH nft 0/5] assorted updates and fixes Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).