lib-smtp: smtp-client-command - Avoid calling the callback for the DATA command durin...

This causes a race condition in which the command object is already freed when
it is returned from smtp_client_command_data_submit(). This scenario occurs when
reading data for the first BDAT command fails.

diff --git a/src/lib-smtp/smtp-client-command.c b/src/lib-smtp/smtp-client-command.c
index fe575cf5851a3dae061a9c81cab8898a32801ee5..65af77cd64ceed947b053a57174ed48362dd4b71 100644 (file)
--- a/src/lib-smtp/smtp-client-command.c
+++ b/src/lib-smtp/smtp-client-command.c
@@ -231,6 +231,13 @@ void smtp_client_command_abort(struct smtp_client_command **_cmd)
        i_assert(!cmd->plug || state <= SMTP_CLIENT_COMMAND_STATE_SUBMITTED);
 
        switch (state) {
+       case SMTP_CLIENT_COMMAND_STATE_NEW:
+               if (cmd->delaying_failure) {
+                       DLLIST_REMOVE(&conn->cmd_fail_list, cmd);
+                       if (conn->cmd_fail_list == NULL)
+                               timeout_remove(&conn->to_cmd_fail);
+               }
+               break;
        case SMTP_CLIENT_COMMAND_STATE_SENDING:
                if (!disconnected) {
                        /* it is being sent; cannot truly abort it now */
@@ -296,6 +303,22 @@ void smtp_client_command_fail_reply(struct smtp_client_command **_cmd,
        if (state >= SMTP_CLIENT_COMMAND_STATE_FINISHED)
                return;
 
+       if (cmd->delay_failure) {
+               i_assert(cmd->delayed_failure == NULL);
+               i_assert(state < SMTP_CLIENT_COMMAND_STATE_SUBMITTED);
+
+               smtp_client_command_debug(cmd, "Fail (delay)");
+
+               cmd->delayed_failure = smtp_reply_clone(cmd->pool, reply);
+               cmd->delaying_failure = TRUE;
+               if (conn->to_cmd_fail == NULL) {
+                       conn->to_cmd_fail = timeout_add_short(0,
+                               smtp_client_commands_fail_delayed, conn);
+               }
+               DLLIST_PREPEND(&conn->cmd_fail_list, cmd);
+               return;
+       }
+
        cmd->callback = NULL;
 
        smtp_client_connection_ref(conn);
@@ -328,6 +351,18 @@ void smtp_client_command_fail(struct smtp_client_command **_cmd,
        smtp_client_command_fail_reply(_cmd, &reply);
 }
 
+static void
+smtp_client_command_fail_delayed(struct smtp_client_command **_cmd)
+{
+       struct smtp_client_command *cmd = *_cmd;
+
+       smtp_client_command_debug(cmd, "Fail delayed");
+
+       i_assert(!cmd->delay_failure);
+       i_assert(cmd->state < SMTP_CLIENT_COMMAND_STATE_FINISHED);
+       smtp_client_command_fail_reply(_cmd, cmd->delayed_failure);
+}
+
 void smtp_client_commands_list_abort(struct smtp_client_command *cmds_list,
                                     unsigned int cmds_list_count)
 {
@@ -389,6 +424,40 @@ void smtp_client_commands_list_fail_reply(
        }
 }
 
+void smtp_client_commands_abort_delayed(struct smtp_client_connection *conn)
+{
+       struct smtp_client_command *cmd;
+
+       timeout_remove(&conn->to_cmd_fail);
+
+       cmd = conn->cmd_fail_list;
+       conn->cmd_fail_list = NULL;
+       while (cmd != NULL) {
+               struct smtp_client_command *cmd_next = cmd->next;
+
+               cmd->delaying_failure = FALSE;
+               smtp_client_command_abort(&cmd);
+               cmd = cmd_next;
+       }
+}
+
+void smtp_client_commands_fail_delayed(struct smtp_client_connection *conn)
+{
+       struct smtp_client_command *cmd;
+
+       timeout_remove(&conn->to_cmd_fail);
+
+       cmd = conn->cmd_fail_list;
+       conn->cmd_fail_list = NULL;
+       while (cmd != NULL) {
+               struct smtp_client_command *cmd_next = cmd->next;
+
+               cmd->delaying_failure = FALSE;
+               smtp_client_command_fail_delayed(&cmd);
+               cmd = cmd_next;
+       }
+}
+
 void smtp_client_command_set_abort_callback(struct smtp_client_command *cmd,
        void (*callback)(void *context), void *context)
 {
@@ -1370,6 +1439,9 @@ smtp_client_command_data_submit_after(
        cmd = cmd_data = smtp_client_command_create(conn,
                flags, callback, context);
 
+       /* protect against race conditions */
+       cmd_data->delay_failure = TRUE;
+
        /* create context in the final command's pool */
        ctx = p_new(cmd->pool, struct _cmd_data_context, 1);
        ctx->conn = conn;
@@ -1432,6 +1504,7 @@ smtp_client_command_data_submit_after(
                _cmd_bdat_send_chunks(ctx, after);
        }
 
+       cmd_data->delay_failure = FALSE;
        return cmd_data;
 }
 

diff --git a/src/lib-smtp/smtp-client-connection.c b/src/lib-smtp/smtp-client-connection.c
index dd40de8fbddbde5c2c0a768d048de2d649d9176c..223c6b5e02435b2cd4db55ac6715003f3fc35674 100644 (file)
--- a/src/lib-smtp/smtp-client-connection.c
+++ b/src/lib-smtp/smtp-client-connection.c
@@ -158,6 +158,7 @@ smtp_client_connection_commands_abort(struct smtp_client_connection *conn)
                                        conn->cmd_wait_list_count);
        smtp_client_commands_list_abort(conn->cmd_send_queue_head,
                                        conn->cmd_send_queue_count);
+       smtp_client_commands_abort_delayed(conn);
 }
 
 static void
@@ -168,6 +169,7 @@ smtp_client_connection_commands_fail_reply(struct smtp_client_connection *conn,
                                             conn->cmd_wait_list_count, reply);
        smtp_client_commands_list_fail_reply(conn->cmd_send_queue_head,
                                             conn->cmd_send_queue_count, reply);
+       smtp_client_commands_fail_delayed(conn);
 }
 
 static void
@@ -1806,6 +1808,7 @@ void smtp_client_connection_disconnect(struct smtp_client_connection *conn)
        timeout_remove(&conn->to_connect);
        timeout_remove(&conn->to_trans);
        timeout_remove(&conn->to_commands);
+       timeout_remove(&conn->to_cmd_fail);
 
        ssl_iostream_destroy(&conn->ssl_iostream);
        if (conn->ssl_ctx != NULL)
@@ -2013,6 +2016,7 @@ void smtp_client_connection_unref(struct smtp_client_connection **_conn)
 
        /* could have been created while already disconnected */
        timeout_remove(&conn->to_commands);
+       timeout_remove(&conn->to_cmd_fail);
 
        smtp_client_connection_debug(conn, "Destroy");
 
@@ -2048,6 +2052,7 @@ void smtp_client_connection_close(struct smtp_client_connection **_conn)
 
        /* could have been created while already disconnected */
        timeout_remove(&conn->to_commands);
+       timeout_remove(&conn->to_cmd_fail);
 
        smtp_client_connection_unref(&conn);
 }
@@ -2074,6 +2079,8 @@ void smtp_client_connection_switch_ioloop(struct smtp_client_connection *conn)
                conn->to_trans = io_loop_move_timeout(&conn->to_trans);
        if (conn->to_commands != NULL)
                conn->to_commands = io_loop_move_timeout(&conn->to_commands);
+       if (conn->to_cmd_fail != NULL)
+               conn->to_cmd_fail = io_loop_move_timeout(&conn->to_cmd_fail);
        connection_switch_ioloop(&conn->conn);
 
        trans = conn->transactions_head;

diff --git a/src/lib-smtp/smtp-client-private.h b/src/lib-smtp/smtp-client-private.h
index 3da54d9c37e98cf9c6c43d1ffb75dc1dab64f4cc..b7021f1879dc33a4e743c1b199bded96496f21b0 100644 (file)
--- a/src/lib-smtp/smtp-client-private.h
+++ b/src/lib-smtp/smtp-client-private.h
@@ -33,6 +33,8 @@ struct smtp_client_command {
        struct istream *stream;
        uoff_t stream_size;
 
+       struct smtp_reply *delayed_failure;
+
        smtp_client_command_callback_t *callback;
        void *context;
 
@@ -49,6 +51,8 @@ struct smtp_client_command {
        bool locked:1;
        bool plug:1;
        bool aborting:1;
+       bool delay_failure:1;
+       bool delaying_failure:1;
 };
 
 struct smtp_client_transaction_mail {
@@ -170,7 +174,7 @@ struct smtp_client_connection {
 
        struct dns_lookup *dns_lookup;
        struct dsasl_client *sasl_client;
-       struct timeout *to_connect, *to_trans, *to_commands;
+       struct timeout *to_connect, *to_trans, *to_commands, *to_cmd_fail;
        struct io *io_cmd_payload;
 
        struct istream *raw_input;
@@ -190,6 +194,8 @@ struct smtp_client_connection {
        /* commands that have been (mostly) sent, waiting for response */
        struct smtp_client_command *cmd_wait_list_head, *cmd_wait_list_tail;
        unsigned int cmd_wait_list_count;
+       /* commands that have failed before submission */
+       struct smtp_client_command *cmd_fail_list;
        /* command sending data stream */
        struct smtp_client_command *cmd_streaming;
 
@@ -247,6 +253,9 @@ void smtp_client_commands_list_fail_reply(
        struct smtp_client_command *cmds_list, unsigned int cmds_list_count,
        const struct smtp_reply *reply);
 
+void smtp_client_commands_abort_delayed(struct smtp_client_connection *conn);
+void smtp_client_commands_fail_delayed(struct smtp_client_connection *conn);
+
 /*
  * Transaction
  */