[nft PATCH 1/2] Reduce signature of do_list_table()

[nft PATCH 1/2] Reduce signature of do_list_table()

[Phil Sutter]

Since commit 16fac7d11bdf5 ("src: use cache infrastructure for rule
objects"), the function does not use the passed 'cmd' object anymore.
Remove it to affirm correctness of a follow-up fix and simplification in
do_list_ruleset().

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/rule.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index a04063f7faf7d..fadd7670d97a2 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1553,8 +1553,7 @@ static int do_command_delete(struct netlink_ctx *ctx, struct cmd *cmd)
 	}
 }
 
-static int do_list_table(struct netlink_ctx *ctx, struct cmd *cmd,
-			 struct table *table)
+static int do_list_table(struct netlink_ctx *ctx, struct table *table)
 {
 	table_print(table, &ctx->nft->output);
 	return 0;
@@ -2188,7 +2187,7 @@ static int do_list_ruleset(struct netlink_ctx *ctx, struct cmd *cmd)
 		cmd->handle.family = table->handle.family;
 		cmd->handle.table.name = table->handle.table.name;
 
-		if (do_list_table(ctx, cmd, table) < 0)
+		if (do_list_table(ctx, table) < 0)
 			return -1;
 	}
 
@@ -2319,7 +2318,7 @@ static int do_command_list(struct netlink_ctx *ctx, struct cmd *cmd)
 	case CMD_OBJ_TABLE:
 		if (!cmd->handle.table.name)
 			return do_list_tables(ctx, cmd);
-		return do_list_table(ctx, cmd, table);
+		return do_list_table(ctx, table);
 	case CMD_OBJ_CHAIN:
 		return do_list_chain(ctx, cmd, table);
 	case CMD_OBJ_CHAINS:
-- 
2.38.0

[nft PATCH 2/2] Avoid a memleak with 'reset rules' command

[Phil Sutter]

Like other 'reset' commands, 'reset rules' also lists the (part of the)
ruleset which was affected to give users a chance to store the zeroed
values. Therefore do_command_reset() calls do_command_list(). This in
turn calls do_list_ruleset() for CMD_OBJ_RULES which wasn't prepared for
values stored in cmd->handle other than a possible family value and thus
freely reused the pointers as scratch area for the do_list_table() call
whiich in the past fetched each table's data directly from kernel.

Meanwhile ruleset listing code has been integrated into the common
caching logic, the 'cmd' pointer became unused by do_list_table(). The
temporary cmd->handle manipulation is not needed anymore, dropping it
prevents a memleak caused by overwriting of allocated table name
pointer.

Fixes: 1694df2de79f3 ("Implement 'reset rule' and 'reset rules' commands")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/rule.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/src/rule.c b/src/rule.c
index fadd7670d97a2..06042239c8437 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -2184,15 +2184,10 @@ static int do_list_ruleset(struct netlink_ctx *ctx, struct cmd *cmd)
 		    table->handle.family != family)
 			continue;
 
-		cmd->handle.family = table->handle.family;
-		cmd->handle.table.name = table->handle.table.name;
-
 		if (do_list_table(ctx, table) < 0)
 			return -1;
 	}
 
-	cmd->handle.table.name = NULL;
-
 	return 0;
 }
 
-- 
2.38.0

Re: [nft PATCH 1/2] Reduce signature of do_list_table()

[Phil Sutter]

On Mon, Mar 20, 2023 at 02:46:58PM +0100, Phil Sutter wrote:
> Since commit 16fac7d11bdf5 ("src: use cache infrastructure for rule
> objects"), the function does not use the passed 'cmd' object anymore.
> Remove it to affirm correctness of a follow-up fix and simplification in
> do_list_ruleset().
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Series applied.