server: Avoid use of uninitialized memory.

change_current_mood() receives an errmsg pointer which the callers
expect to be initialized with an error string if (and only if) the
function returns negative.

However, most error paths miss to initialize the pointer which results
in undefined behaviour in the caller which attempts to free(3)
uninitialized memory. The gcc AddressSanitizer and valgrind both
catch this:

gcc:

	==14788==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x081af250 in thread T0

valgrind:

	==4410== Invalid free() / delete / delete[] / realloc()

The bug was introduced half a year ago when version 2 moods were
introduced in commit 3d3a2f50.

diff --git a/mood.c b/mood.c
index ae8364a29ea7b4028ebbe2bfb812ef6592b402c0..a63d4d2af5d10d7b64c319d915e00b9b7ea62e89 100644 (file)
--- a/mood.c
+++ b/mood.c
@@ -418,10 +418,13 @@ static int load_mood(const struct osl_row *mood_row, struct mood **m,
 
        *m = NULL;
        ret = mood_get_name_and_def_by_row(mood_row, &mood_name, &mood_def);
 
        *m = NULL;
        ret = mood_get_name_and_def_by_row(mood_row, &mood_name, &mood_def);

-       if (ret < 0)
+       if (ret < 0) {
+               if (errmsg)
+                       *errmsg = make_message(
+                               "could not read mood definition");

                return ret;
                return ret;

-       if (!*mood_name)
-               return -E_DUMMY_ROW;
+       }
+       assert(*mood_name);

        mlpd.m = alloc_new_mood(mood_name);
        ret = for_each_line(FELF_READ_ONLY, mood_def.data, mood_def.size,
                parse_mood_line, &mlpd);
        mlpd.m = alloc_new_mood(mood_name);
        ret = for_each_line(FELF_READ_ONLY, mood_def.data, mood_def.size,
                parse_mood_line, &mlpd);


@@ -876,7 +879,9 @@ int change_current_mood(const char *mood_name, char **errmsg)
                };
                ret = osl(osl_get_row(moods_table, BLOBCOL_NAME, &obj, &row));
                if (ret < 0) {
                };
                ret = osl(osl_get_row(moods_table, BLOBCOL_NAME, &obj, &row));
                if (ret < 0) {

-                       PARA_NOTICE_LOG("no such mood: %s\n", mood_name);
+                       if (errmsg)
+                               *errmsg = make_message("no such mood: %s",
+                                       mood_name);

                        return ret;
                }
                ret = load_mood(row, &m, errmsg);
                        return ret;
                }
                ret = load_mood(row, &m, errmsg);


@@ -891,13 +896,20 @@ int change_current_mood(const char *mood_name, char **errmsg)
        aa.m = current_mood;
        PARA_NOTICE_LOG("computing statistics of admissible files\n");
        ret = audio_file_loop(&aa, add_if_admissible);
        aa.m = current_mood;
        PARA_NOTICE_LOG("computing statistics of admissible files\n");
        ret = audio_file_loop(&aa, add_if_admissible);

-       if (ret < 0)
+       if (ret < 0) {
+               if (errmsg)
+                       *errmsg = make_message("audio file loop failed");

                return ret;
                return ret;

+       }

        for (i = 0; i < statistics.num; i++) {
                struct admissible_file_info *a = aa.array + i;
                ret = add_to_score_table(a->aft_row, a->score);
        for (i = 0; i < statistics.num; i++) {
                struct admissible_file_info *a = aa.array + i;
                ret = add_to_score_table(a->aft_row, a->score);

-               if (ret < 0)
+               if (ret < 0) {
+                       if (errmsg)
+                               *errmsg = make_message(
+                                       "could not add row to score table");

                        goto out;
                        goto out;

+               }

        }
        log_statistics();
        ret = statistics.num;
        }
        log_statistics();
        ret = statistics.num;