app_voicemail_odbc: fix msgnum race and crash on failed STORE

app_voicemail_odbc: fix msgnum race and crash on failed STORE

Two concurrent callers leaving voicemail to the same mailbox could be
assigned the same msgnum because ast_unlock_path() was called before
STORE(), allowing a second thread to read the same LAST_MSG_INDEX()
before the first INSERT committed. The losing thread got a duplicate
key error, but execution continued into notify_new_message() ->
RETRIEVE() because the STORE() return value was not checked.
RETRIEVE() then fetched the winning thread's DB row, mmap'd its blob
size against the locally truncated file, and crashed with SIGBUS.

Hold the path lock through STORE() and bail out on failure.

Fixes: #1653

diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index 68af361e73ff0468757982463bca3dc496391393..6be1cf6a7acc7ccd56226dc8f4810f82c278f337 100644 (file)
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -7483,16 +7483,34 @@ static int leave_voicemail(struct ast_channel *chan, char *ext, struct leave_vm_
                                        if (chmod(txtfile, VOICEMAIL_FILE_MODE) < 0)
                                                ast_log(AST_LOG_ERROR, "Couldn't set permissions on voicemail text file %s: %s", txtfile, strerror(errno));
 
-                                       ast_unlock_path(dir);
                                        if (ast_check_realtime("voicemail_data")) {
                                                snprintf(tmpdur, sizeof(tmpdur), "%d", duration);
                                                ast_update_realtime("voicemail_data", "filename", tmptxtfile, "filename", fn, "duration", tmpdur, SENTINEL);
                                        }
                                        /* We must store the file first, before copying the message, because
                                         * ODBC storage does the entire copy with SQL.
+                                        * Hold the path lock until after STORE so that concurrent callers
+                                        * cannot read the same LAST_MSG_INDEX before the INSERT is committed.
                                         */
                                        if (ast_fileexists(fn, NULL, NULL) > 0) {
+#ifdef ODBC_STORAGE
+                                               int store_failed;
+                                               store_failed = odbc_store_message(dir, vmu->mailbox, vmu->context, msgnum);
+                                               ast_unlock_path(dir);
+                                               if (store_failed) {
+                                                       ast_log(LOG_ERROR, "Failed to store voicemail for %s/%s msgnum %d, skipping notification\n", vmu->context, vmu->mailbox, msgnum);
+                                                       if (ast_check_realtime("voicemail_data")) {
+                                                               ast_destroy_realtime("voicemail_data", "filename", fn, SENTINEL);
+                                                       }
+                                                       pbx_builtin_setvar_helper(chan, "VMSTATUS", "FAILED");
+                                                       goto leave_vm_out;
+                                               }
+#else
+                                               ast_unlock_path(dir);
                                                SCOPE_CALL(-1, STORE, dir, vmu->mailbox, vmu->context, msgnum, chan, vmu, fmt, duration, vms, flag, msg_id);
+#endif
+                                       } else {
+                                               ast_unlock_path(dir);
                                        }
 
                                        /* Are there to be more recipients of this message? */