setvar & co.: variables not incrementing properly in threaded workers

[celesteking]

This may be related to #1224 , but I've got it enabled, so it must've been something else.

The var increment is "skipping". UPDATE_COUNTER is updated normally (internal var), while user-defined thevar isn't incrementing as it should. It works totally fine when requests are sent in series (ab -c 1)

ab -n 1 xx/x.php

# modsec-sdbm-util -d -u  /var/cpanel/secdatadir/nobody-resource.pag 
Opening file: /var/cpanel/secdatadir/nobody-resource.pag
Database ready to be used.
Key: "192.168.1.1", Value len: 240
 - ModSecurity variables:
                  __expire_KEY: 1590438405
                           KEY: 192.168.1.1
                       TIMEOUT: 3600
                         __key: 192.168.1.1
                        __name: resource
                   CREATE_TIME: 1590434805
                UPDATE_COUNTER: 1
                        thevar: 1
               __expire_thevar: 1590434865
              LAST_UPDATE_TIME: 1590434805

ab -n 50 -c 5 http://xx/x.php

 - ModSecurity variables:
                  __expire_KEY: 1590438413
                           KEY: 192.168.1.1
                       TIMEOUT: 3600
                         __key: 192.168.1.1
                        __name: resource
                   CREATE_TIME: 1590434805
                UPDATE_COUNTER: 51
                        thevar: 15
               __expire_thevar: 1590434865
              LAST_UPDATE_TIME: 1590434813

The ruleset:

SecAction "id:18000,\
    phase:1,nolog,pass,\
    t:none,\
    initcol:resource=%{remote_addr},\
    setvar:tx.real_ip=%{remote_addr},\
    setvar:tx.request_filename=%{REQUEST_FILENAME},"

SecRule SCRIPT_BASENAME "@eq x.php" "id:18420,\
    phase:5,nolog,pass,\
    capture,t:none,t:lowercase,\
    setvar:'tx.hit=1'"

SecRule TX:hit "@eq 1"  "id:18422,\
        phase:5,nolog,pass,t:none,\
        skipAfter:ENDIT,\
        chain"

        SecRule &RESOURCE:thevar "@eq 0" "setvar:'resource.thevar=1', expirevar:'resource.thevar=60'"

SecRule TX:hit "@eq 1"  "id:18423,\
        phase:5,nolog,pass,t:none,\
        chain"

        SecRule RESOURCE:thevar "@gt 0" "setvar:'resource.thevar=+1'"

SecMarker "ENDIT"

Compile flags:

./configure --build=x86_64-redhat-linux-gnu --host=x86_64-redhat-linux-gnu --program-prefix= --disable-dependency-tracking --prefix=/usr --exec-prefix=/usr --bindir=/usr/bin --sbindir=/usr/sbin --sysconfdir=/etc --datadir=/usr/share --includedir=/usr/include --libdir=/usr/lib64 --libexecdir=/usr/libexec --localstatedir=/var --sharedstatedir=/var/lib --mandir=/usr/share/man --infodir=/usr/share/info --enable-pcre-match-limit=1000000 --enable-pcre-match-limit-recursion=1000000 --with-apr=/opt/cpanel/ea-apr16 --with-apu=/opt/cpanel/ea-apr16 --with-apxs=/usr/bin/apxs --with-curl=/opt/cpanel/libcurl --with-libxml=/opt/cpanel/ea-libxml2 --enable-collection-global-lock

ea-apache24-mod_security2-2.9.3-5 cpanel-sourced, but I've added --enable-collection-global-lock as you see.

# httpd -V
Server version: Apache/2.4.43 (cPanel)
Server built:   May  5 2020 18:56:17
Server's Module Magic Number: 20120211:92
Server loaded:  APR 1.7.0, APR-UTIL 1.6.1
Compiled using: APR 1.7.0, APR-UTIL 1.6.1
Architecture:   64-bit
Server MPM:     worker
  threaded:     yes (fixed thread count)
    forked:     yes (variable process count)
Server compiled with....
 -D APR_HAS_SENDFILE
 -D APR_HAS_MMAP
 -D APR_HAVE_IPV6 (IPv4-mapped addresses disabled)
 -D APR_USE_SYSVSEM_SERIALIZE
 -D APR_USE_PTHREAD_SERIALIZE
 -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
 -D APR_HAS_OTHER_CHILD
 -D AP_HAVE_RELIABLE_PIPED_LOGS
 -D DYNAMIC_MODULE_LIMIT=256
 -D HTTPD_ROOT="/etc/apache2"
 -D SUEXEC_BIN="/usr/sbin/suexec"
 -D DEFAULT_PIDLOG="/var/run/apache2/httpd.pid"
 -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
 -D DEFAULT_ERRORLOG="logs/error_log"
 -D AP_TYPES_CONFIG_FILE="conf/mime.types"
 -D SERVER_CONFIG_FILE="conf/httpd.conf"

[celesteking]

Well this is screwed up -- it sustains threading with IP collection and doesn't with RESOURCE col. What the hell, guys?

[zimmerle]

The variable values computation in between the different threads are made by random factor after each request. Increase a counter is ok, but, it will not represent the total amount of requests and the value will be different for each thread/worker/process. Having said that, checking the counter value needs to consider this factor.

[celesteking]

What are you talking about? There's a global lock around those operations, they were supposed to be atomic.
Secondly, why on earth is IP collection incrementing atomically as expected while RESOURCE is not? What's so special about it?