From: Stephan Bosch <stephan.bosch@dovecot.fi>
Date: Sat, 23 Dec 2017 21:04:13 +0000 (+0100)
Subject: lib-smtp: client: Make smtp_client_connection_commands_fail_reply() more reliable... 
X-Git-Tag: 2.3.1~67
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0ac8caa0e1348517b30ee041e160d9903a7f159e;p=thirdparty%2Fdovecot%2Fcore.git

lib-smtp: client: Make smtp_client_connection_commands_fail_reply() more reliable by copying the command lists.

Copy the current lists of queued and waiting commands and reference each command before calling smtp_client_command_fail_reply().
Failing one command can cause other dependent commands to be aborted (in a transaction or from submission service), which could have caused trouble in this function.
Problems would likely occur at connection disconnect.
---

diff --git a/src/lib-smtp/smtp-client-command.c b/src/lib-smtp/smtp-client-command.c
index 42477f9f41..8ea5dcce38 100644
--- a/src/lib-smtp/smtp-client-command.c
+++ b/src/lib-smtp/smtp-client-command.c
@@ -313,6 +313,37 @@ void smtp_client_command_fail(struct smtp_client_command **_cmd,
 	smtp_client_command_fail_reply(_cmd, &reply);
 }
 
+void smtp_client_commands_list_fail_reply(
+	struct smtp_client_command *cmds_list, unsigned int cmds_list_count,
+	const struct smtp_reply *reply)
+{
+	struct smtp_client_command *cmd;
+	ARRAY(struct smtp_client_command *) cmds_arr;
+	struct smtp_client_command **cmds;
+	unsigned int count, i;
+
+	if (cmds_list == NULL)
+		return;
+	i_assert(cmds_list_count > 0);
+
+	/* copy the array and reference the commands to be robust against more
+	   than one command disappearing from the list */
+	t_array_init(&cmds_arr, cmds_list_count);
+	for (cmd = cmds_list; cmd != NULL; cmd = cmd->next) {
+		smtp_client_command_ref(cmd);
+		array_append(&cmds_arr, &cmd, 1);
+	}
+
+	cmds = array_get_modifiable(&cmds_arr, &count);
+	for (i = 0; i < count; i++) {
+		cmd = cmds[i];
+		/* fail the reply */
+		smtp_client_command_fail_reply(&cmds[i], reply);
+		/* drop our reference */
+		smtp_client_command_unref(&cmd);
+	}
+}
+
 void smtp_client_command_set_abort_callback(struct smtp_client_command *cmd,
 	void (*callback)(void *context), void *context)
 {
diff --git a/src/lib-smtp/smtp-client-connection.c b/src/lib-smtp/smtp-client-connection.c
index a361794bde..d74c8555ad 100644
--- a/src/lib-smtp/smtp-client-connection.c
+++ b/src/lib-smtp/smtp-client-connection.c
@@ -145,23 +145,10 @@ static void
 smtp_client_connection_commands_fail_reply(struct smtp_client_connection *conn,
 	const struct smtp_reply *reply)
 {
-	struct smtp_client_command *cmd, *cmd_next = NULL;
-
-	cmd = conn->cmd_wait_list_head;
-	while (cmd != NULL) {
-		i_assert(conn->cmd_wait_list_count > 0);
-		cmd_next = cmd->next;
-		smtp_client_command_fail_reply(&cmd, reply);
-		cmd = cmd_next;
-	}
-
-	cmd = conn->cmd_send_queue_head;
-	while (cmd != NULL) {
-		i_assert(conn->cmd_send_queue_count > 0);
-		cmd_next = cmd->next;
-		smtp_client_command_fail_reply(&cmd, reply);
-		cmd = cmd_next;
-	}
+	smtp_client_commands_list_fail_reply(conn->cmd_wait_list_head,
+					     conn->cmd_wait_list_count, reply);
+	smtp_client_commands_list_fail_reply(conn->cmd_send_queue_head,
+					     conn->cmd_send_queue_count, reply);
 }
 
 static void
diff --git a/src/lib-smtp/smtp-client-private.h b/src/lib-smtp/smtp-client-private.h
index f682765153..c399d8b120 100644
--- a/src/lib-smtp/smtp-client-private.h
+++ b/src/lib-smtp/smtp-client-private.h
@@ -190,10 +190,14 @@ void smtp_client_command_free(struct smtp_client_command *cmd);
 int smtp_client_command_send_more(struct smtp_client_connection *conn);
 int smtp_client_command_input_reply(struct smtp_client_command *cmd,
 				    const struct smtp_reply *reply);
+
 void smtp_client_command_fail(struct smtp_client_command **_cmd,
 			      unsigned int status, const char *error);
 void smtp_client_command_fail_reply(struct smtp_client_command **_cmd,
 				    const struct smtp_reply *reply);
+void smtp_client_commands_list_fail_reply(
+	struct smtp_client_command *cmds_list, unsigned int cmds_list_count,
+	const struct smtp_reply *reply);
 
 /*
  * Transaction