From 87d1a4edde9ffaea5732955c1899bd523c6297b5 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Thu, 23 Dec 2021 23:51:42 +0100 Subject: [PATCH] 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 bdeaa66..4715ba7 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 d13ecd8..d3388d9 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";