Rsyslog: Potential data race in processPacket

[ITWOI]

Environment

• rsyslog version: Master branch
• platform: Ubuntu 16.04
• for configuration questions/issues, include rsyslog.conf:

Hi all,

Our bug scanner has reported a data race issue from imudp.c#L429 to imudp.c#L430

Followings are code snippets.

static rsRetVal
processPacket(...)
{
    ...
    if(bDoACLCheck) {
        socklen = sizeof(struct sockaddr_storage);
        if(net.CmpHost(frominet, frominetPrev, socklen) != 0) {
            ...
            if(*pbIsPermitted == 0) {
                DBGPRINTF("msg is not from an allowed sender\n");
                if(glbl.GetOption_DisallowWarning) {
                    time_t tt;
                    datetime.GetTime(&tt);
                    if(tt > ttLastDiscard + 60) {
                        ttLastDiscard = tt;
                        errmsg.LogError(0, NO_ERRCODE,
                            "UDP message from disallowed sender discarded");
                    }
                }
            }
        }
    } 
    ...
}

ttLastDiscard is a static variable and the function processPacket could be executed parallelly. Therefore, when two threads reach the if(tt > ttLastDiscard + 60) simultaneously, they may be all true and then begin to assign the variable ttLastDiscard. However, they could corrupt the ttLastDiscard, leading to confused information.

A possible call trace could be as follows:

BEGINrunInput creates threads that execute function wrkr. Then wrkr calls rcvMainLoop(pWrkr). At the last loop of rcvMainLoop, it calls processSocket(pWrkr, currEvt[i].data.ptr, &frominetPrev, &bIsPermitted);.

SourceBrella Inc.,
Yu

[rgerhards]

Thx for the report, we will look as fast as possible at it in depth - but it may still take a bit as I am currently busy with other parts of the project.

[rgerhards]

Yeah, it's a bug, but the cure is actually to depend on the rate limiter for rsyslog internal messages. It was most probably not yet available when this code was crafted. Also, we should consider adding an impstats counter for this case. Would probably be more useful.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.