[commit] master: fix invalid data accesses in proxy driver callbacks

[Oswald B. <os...@us...>]

commit 87d1a4edde9ffaea5732955c1899bd523c6297b5
Author: Oswald Buddenhagen <os...@us...>
Date:   Thu Dec 23 23:51:42 2021 +0100

    fix invalid data accesses in proxy driver callbacks
    
    all printing of auxiliary callback arguments must be conditional on the
    command having actually succeeded. this affected fetch_msg() most, which
    outright crashed due to a null pointer deref.
    
    to fix this for good, we automate the generation of the status printing
    and checking. as a side effect, this fixes the fetch_msg() callback not
    printing the status at all.
    
    amends 4cc5ad5a.

 src/drv_proxy.c      | 55 ++++++++++++++++++++++++++++----------------
 src/drv_proxy_gen.pl | 29 ++++++++++++++++++++---
 2 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/src/drv_proxy.c b/src/drv_proxy.c
index bdeaa66d..4715ba7b 100644
--- a/src/drv_proxy.c
+++ b/src/drv_proxy.c
@@ -172,6 +172,28 @@ static @type@proxy_@name@( store_t *gctx@decl_args@ )
 }
 //# END
 
+//# TEMPLATE CALLBACK_VOID
+	debug( "%s[% 2d] Callback enter @name@\n", ctx->label, cmd->tag );
+	@print_cb_args@
+//# END
+//# TEMPLATE CALLBACK_STS
+	debug( "%s[% 2d] Callback enter @name@, sts=%d\n", ctx->label, cmd->tag, sts );
+//# END
+//# TEMPLATE CALLBACK_STS_PRN
+	debug( "%s[% 2d] Callback enter @name@, sts=%d\n", ctx->label, cmd->tag, sts );
+	if (sts == DRV_OK) {
+		@print_cb_args@
+	}
+//# END
+//# TEMPLATE CALLBACK_STS_FMT
+	if (sts == DRV_OK) {
+		debug( "%s[% 2d] Callback enter @name@, sts=" stringify(DRV_OK) "@print_fmt_cb_args@\n", ctx->label, cmd->tag@print_pass_cb_args@ );
+		@print_cb_args@
+	} else {
+		debug( "%s[% 2d] Callback enter @name@, sts=%d\n", ctx->label, cmd->tag, sts );
+	}
+//# END
+
 //# TEMPLATE CALLBACK
 typedef union {
 	gen_cmd_t gen;
@@ -189,8 +211,7 @@ proxy_@name@_cb( @decl_cb_args@void *aux )
 	@name@_cmd_t *cmd = (@name@_cmd_t *)aux;
 	proxy_store_t *ctx = cmd->ctx;
 
-	debug( "%s[% 2d] Callback enter @name@@print_fmt_cb_args@\n", ctx->label, cmd->tag@print_pass_cb_args@ );
-	@print_cb_args@
+	@print_cb_args_tpl@
 	cmd->callback( @pass_cb_args@cmd->callback_aux );
 	debug( "%s[% 2d] Callback leave @name@\n", ctx->label, cmd->tag );
 	proxy_cmd_done( &cmd->gen );
@@ -225,10 +246,8 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
 //# UNDEFINE list_store_print_fmt_cb_args
 //# UNDEFINE list_store_print_pass_cb_args
 //# DEFINE list_store_print_cb_args
-	if (sts == DRV_OK) {
-		for (string_list_t *box = boxes; box; box = box->next)
-			debug( "  %s\n", box->string );
-	}
+	for (string_list_t *box = boxes; box; box = box->next)
+		debug( "  %s\n", box->string );
 //# END
 
 //# DEFINE prepare_load_box_print_fmt_args , opts=%s
@@ -249,23 +268,19 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
 		debug( "\n" );
 	}
 //# END
-//# DEFINE load_box_print_fmt_cb_args , sts=%d, total=%d, recent=%d
-//# DEFINE load_box_print_pass_cb_args , sts, total_msgs, recent_msgs
+//# DEFINE load_box_print_fmt_cb_args , total=%d, recent=%d
+//# DEFINE load_box_print_pass_cb_args , total_msgs, recent_msgs
 //# DEFINE load_box_print_cb_args
-	if (sts == DRV_OK) {
-		for (message_t *msg = msgs; msg; msg = msg->next)
-			debug( "  uid=%-5u flags=%-4s size=%-6u tuid=%." stringify(TUIDL) "s\n",
-			       msg->uid, (msg->status & M_FLAGS) ? fmt_flags( msg->flags ).str : "?", msg->size, *msg->tuid ? msg->tuid : "?" );
-	}
+	for (message_t *msg = msgs; msg; msg = msg->next)
+		debug( "  uid=%-5u flags=%-4s size=%-6u tuid=%." stringify(TUIDL) "s\n",
+		       msg->uid, (msg->status & M_FLAGS) ? fmt_flags( msg->flags ).str : "?", msg->size, *msg->tuid ? msg->tuid : "?" );
 //# END
 
-//# DEFINE find_new_msgs_print_fmt_cb_args , sts=%d
-//# DEFINE find_new_msgs_print_pass_cb_args , sts
+//# UNDEFINE find_new_msgs_print_fmt_cb_args
+//# UNDEFINE find_new_msgs_print_pass_cb_args
 //# DEFINE find_new_msgs_print_cb_args
-	if (sts == DRV_OK) {
-		for (message_t *msg = msgs; msg; msg = msg->next)
-			debug( "  uid=%-5u tuid=%." stringify(TUIDL) "s\n", msg->uid, msg->tuid );
-	}
+	for (message_t *msg = msgs; msg; msg = msg->next)
+		debug( "  uid=%-5u tuid=%." stringify(TUIDL) "s\n", msg->uid, msg->tuid );
 //# END
 
 //# DEFINE fetch_msg_print_fmt_args , uid=%u, want_flags=%s, want_date=%s
@@ -273,7 +288,7 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
 //# DEFINE fetch_msg_print_fmt_cb_args , flags=%s, date=%lld, size=%u
 //# DEFINE fetch_msg_print_pass_cb_args , fmt_flags( cmd->data->flags ).str, (long long)cmd->data->date, cmd->data->len
 //# DEFINE fetch_msg_print_cb_args
-	if (sts == DRV_OK && (DFlags & DEBUG_DRV_ALL)) {
+	if (DFlags & DEBUG_DRV_ALL) {
 		printf( "%s=========\n", cmd->ctx->label );
 		fwrite( cmd->data->data, cmd->data->len, 1, stdout );
 		printf( "%s=========\n", cmd->ctx->label );
diff --git a/src/drv_proxy_gen.pl b/src/drv_proxy_gen.pl
index d13ecd88..d3388d98 100755
--- a/src/drv_proxy_gen.pl
+++ b/src/drv_proxy_gen.pl
@@ -126,6 +126,7 @@ for (@ptypes) {
 	}
 	push @cmd_table, "proxy_$cmd_name";
 	next if (defined($special{$cmd_name}));
+	my $inc_tpl = "";
 	my %replace;
 	$replace{'name'} = $cmd_name;
 	$replace{'type'} = $cmd_type;
@@ -139,9 +140,16 @@ for (@ptypes) {
 			my $cmd_cb_args = $1;
 			$replace{'decl_cb_args'} = $cmd_cb_args;
 			$replace{'pass_cb_args'} = make_args($cmd_cb_args);
-			my $cmd_print_cb_args = $cmd_cb_args =~ s/(.*), $/, $1/r;
-			$replace{'print_pass_cb_args'} = make_args($cmd_print_cb_args);
-			$replace{'print_fmt_cb_args'} = make_format($cmd_print_cb_args);
+			if (length($cmd_cb_args)) {
+				my $r_cmd_cb_args = $cmd_cb_args;
+				$r_cmd_cb_args =~ s/^int sts, // or die("Callback arguments of $cmd_name don't start with sts.\n");
+				$r_cmd_cb_args =~ s/^(.*), $/, $1/;
+				$replace{'print_pass_cb_args'} = make_args($r_cmd_cb_args);
+				$replace{'print_fmt_cb_args'} = make_format($r_cmd_cb_args);
+				$inc_tpl = 'CALLBACK_STS';
+			} else {
+				$inc_tpl = 'CALLBACK_VOID';
+			}
 
 			$pass_args = make_args($cmd_args);
 			$pass_args =~ s/([^, ]+)/cmd->$1/g;
@@ -172,6 +180,21 @@ for (@ptypes) {
 	}
 	my %used;
 	my $text = $templates{$template};
+	if ($inc_tpl) {
+		if ($inc_tpl eq 'CALLBACK_STS') {
+			if ($replace{print_fmt_cb_args}) {
+				$inc_tpl .= '_FMT';
+			} else {
+				if ($replace{print_cb_args}) {
+					$inc_tpl .= '_PRN';
+				}
+				# These may be defined but empty; that's not an error.
+				delete $replace{print_fmt_cb_args};
+				delete $replace{print_pass_cb_args};
+			}
+		}
+		$text =~ s/^\t\@print_cb_args_tpl\@\n/$templates{$inc_tpl}/sm;
+	}
 	$text =~ s/^(\h*)\@(\w+)\@\n/$used{$2} = 1; indent($replace{$2} \/\/ "", $1)/smeg;
 	$text =~ s/\@(\w+)\@/$used{$1} = 1; $replace{$1} \/\/ ""/eg;
 	print $outh $text."\n";