andreasschulze / signing-milter

signing-milter enables you to s/mime sign an ordinary mail while passing a MTA
https://signing-milter.org
GNU General Public License v2.0
2 stars 3 forks source link

memory disruption - multipe threads #7

Open pzawora opened 2 years ago

pzawora commented 2 years ago

Function dict_lookup is not thread safety, but its called from multiple threads. Function operate on global object dict_signingtable. pointer to the allocated memory is returned to the thread, while other thread can reallocate memory or strncpy new data.

As a result I can find in logs:

load_pem_cert: BIO_read_filename() failed
error: ctxdata_setup: loading certificate  failed
callback_header: ctxdata_setup() failed: rc=3, headerf=, headerv=, file=X-Signer

probability of error depend on the number of threads. In single thread - there is no errors, 400+ threads >= one error /per second.

Enviromement: gentoo, postfix with 1024 smtpd workers, dev-db/tinycdb-0.77-r2

working workaround: ensure single thread access to the method.

    const char * res;
    pthread_mutex_lock(&mutCdb);
    res = dict_lookup_internal(dict,key);
    int currentSize = strlen(res) + 1;
    if ( currentSize > buffSize) {
            logmsg(LOG_ERR, "DICT LOOKUP VALUE TOO LONG (%d). must be  < %d",currentSize,buffSize );
            pthread_mutex_unlock(&mutCdb);
            exit(EX_SOFTWARE);
    }
    strcpy(buffer,res);
    pthread_mutex_unlock(&mutCdb);
    return buffer;
}

It is enough to look at code of original dict_lookup - to see that function returns pointer to allocated memory, while multiple thread can operate on the SAME pointer (ie strncpy) , even more - reallocate it. Calling function can see different string in time, it could be also free()-zed memory. Based on the returned string - BIO is trying to open cert or key file.

Also it causes memory leak,

More details in direct email On 7/12/2021 12:11 PM,

andreasschulze commented 2 years ago

Thanks for the report and detailed analysis. I'll try to understand and reproduce the issue. Reviewing and integration of your proposed solution will take some spare time...