Skip to content

mxq_job: Fix invalid pointer deref on error path #84

Merged
merged 1 commit into from
Apr 15, 2020
Merged

Commits on Apr 15, 2020

  1. mxq_job: Fix invalid pointer deref on error path

    I've got this from valgrind:
    
        2020-04-15 16:11:00 +0200 mxqd[6617]: mxqd mxqd.c:2600:main(): DEBUG: group_cnt=36 :: 36 Groups loaded
        2020-04-15 16:11:00 +0200 mxqd[6617]: mxqd mxqd.c:1343:start_user(): DEBUG:  user=buczek(125) slots_to_start=11 :: trying to start jobs for user.
        2020-04-15 16:11:00 +0200 mxqd[6617]: mxqd mxqd.c:1370:start_user():   group=buczek(125):335148 slots_to_start=11 slots_per_job=11 :: trying to start job for group.
        ==6617== Conditional jump or move depends on uninitialised value(s)
        ==6617==    at 0x40318FB: free (vg_replace_malloc.c:540)
        ==6617==    by 0x415307: mxq_select_job_from_group (mxq_job.c:324)
        ==6617==    by 0x41545F: mxq_assign_job_from_group_to_daemon (mxq_job.c:355)
        ==6617==    by 0x416849: mxq_load_job_from_group_for_daemon (mxq_job.c:740)
        ==6617==    by 0x40715F: start_job (mxqd.c:1188)
        ==6617==    by 0x407B82: start_user (mxqd.c:1372)
        ==6617==    by 0x407CC1: start_user_with_least_running_global_slot_count (mxqd.c:1412)
        ==6617==    by 0x40B450: main (mxqd.c:2609)
        ==6617==
        2020-04-15 16:11:00 +0200 mxqd[6617]: mxqd mxq_job.c:746:mxq_load_job_from_group_for_daemon(): WARNING:   group_id=335148 :: mxq_assign_job_from_group_to_daemon(): No matching job found - maybe another server was a bit faster. ;)
    
    The pointer freed at the end of `mxq_select_job_from_group()` is supposed
    to be allocated by `mx_mysql_do_statement()`. However, the pointer is
    only allocated in the common case, where the routine delivers a result:
    
        if (result && result->count && num_rows) {
            tmpdata = mx_calloc_forever(num_rows, size);
            ...
            *to = tmpdata;
    
    In the unusual case that there was no result from the query (as above,
    because other servers already started all pending jobs) or in error
    cases, `job_id_out` might be uninitialized and the `free()` might
    is invalid.
    
    Make sure, we don't try to free via an unitialized pointer. Do not decide
    based on the return value of `mx_mysql_do_statement()`, because we don't
    want to duplicate the complicated logic in the if stated quited above to
    be duplicated in the caller.
    donald committed Apr 15, 2020
    Configuration menu
    Copy the full SHA
    e2aa076 View commit details
    Browse the repository at this point in the history