BUG/MEDIUM: stktable: fix missing lock on some table converters

In 819fc6f563
("MEDIUM: threads/stick-tables: handle multithreads on stick tables"),
sample fetch and action functions were properly guarded with stksess
read/write locks for read and write operations respectively, but the
sample_conv_table functions leveraged by "table_*" converters were
overlooked.

This bug was not known to cause issues in existing deployments yet (at
least it was not reported), but due to its nature it can theorically lead
to inconsistent values being reported by "table_*" converters if the value
is being updated by another thread in parallel.

It should be backported to all stable versions.

[ada: for versions < 3.0, glitch_cnt and glitch_rate samples should be
 ignored as they first appeared in 3.0]

diff --git a/src/stick_table.c b/src/stick_table.c
index 122870059e0040dfb129811e692cc98d2e804f2e..852995a1a5f7f5c60b2f260ab729a915f3779100 100644 (file)
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -1817,10 +1817,15 @@ static int sample_conv_table_bytes_in_rate(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_IN_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = (uint64_t)read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                                  t->data_arg[STKTABLE_DT_BYTES_IN_RATE].u) * t->brates_factor;
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1854,9 +1859,14 @@ static int sample_conv_table_conn_cnt(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1890,9 +1900,14 @@ static int sample_conv_table_conn_cur(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_CUR);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -1926,10 +1941,15 @@ static int sample_conv_table_conn_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_CONN_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_CONN_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2039,10 +2059,14 @@ static int sample_conv_table_bytes_out_rate(const struct arg *arg_p, struct samp
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_OUT_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = (uint64_t)read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                                  t->data_arg[STKTABLE_DT_BYTES_OUT_RATE].u) * t->brates_factor;
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
 
        stktable_release(t, ts);
        return !!ptr;
@@ -2077,9 +2101,14 @@ static int sample_conv_table_glitch_cnt(const struct arg *arg_p, struct sample *
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GLITCH_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2113,10 +2142,15 @@ static int sample_conv_table_glitch_rate(const struct arg *arg_p, struct sample
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GLITCH_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GLITCH_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2153,9 +2187,14 @@ static int sample_conv_table_gpt(const struct arg *arg_p, struct sample *smp, vo
                return 1;
 
        ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPT, idx);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2192,9 +2231,14 @@ static int sample_conv_table_gpt0(const struct arg *arg_p, struct sample *smp, v
        if (!ptr)
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPT, 0);
 
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2231,9 +2275,14 @@ static int sample_conv_table_gpc(const struct arg *arg_p, struct sample *smp, vo
                return 1;
 
        ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC, idx);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2270,10 +2319,15 @@ static int sample_conv_table_gpc_rate(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC_RATE, idx);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GPC_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2312,9 +2366,14 @@ static int sample_conv_table_gpc0(const struct arg *arg_p, struct sample *smp, v
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC, 0);
        }
 
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2348,15 +2407,25 @@ static int sample_conv_table_gpc0_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GPC0_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GPC0_RATE].u);
+
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
        else {
                /* fallback on the gpc array */
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC_RATE, 0);
-               if (ptr)
+               if (ptr) {
+                       HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                        smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                                t->data_arg[STKTABLE_DT_GPC_RATE].u);
+
+                       HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+               }
        }
 
        stktable_release(t, ts);
@@ -2397,9 +2466,14 @@ static int sample_conv_table_gpc1(const struct arg *arg_p, struct sample *smp, v
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC, 1);
        }
 
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2433,15 +2507,25 @@ static int sample_conv_table_gpc1_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_GPC1_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_GPC1_RATE].u);
+
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
        else {
                /* fallback on the gpc array */
                ptr = stktable_data_ptr_idx(t, ts, STKTABLE_DT_GPC_RATE, 1);
-               if (ptr)
+               if (ptr) {
+                       HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                        smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                                t->data_arg[STKTABLE_DT_GPC_RATE].u);
+
+                       HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+               }
        }
 
        stktable_release(t, ts);
@@ -2477,9 +2561,14 @@ static int sample_conv_table_http_err_cnt(const struct arg *arg_p, struct sample
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2513,10 +2602,15 @@ static int sample_conv_table_http_err_rate(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_ERR_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_HTTP_ERR_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2550,9 +2644,14 @@ static int sample_conv_table_http_fail_cnt(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_FAIL_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2586,10 +2685,15 @@ static int sample_conv_table_http_fail_rate(const struct arg *arg_p, struct samp
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_FAIL_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_HTTP_FAIL_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2623,9 +2727,14 @@ static int sample_conv_table_http_req_cnt(const struct arg *arg_p, struct sample
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2659,10 +2768,15 @@ static int sample_conv_table_http_req_rate(const struct arg *arg_p, struct sampl
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_HTTP_REQ_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_HTTP_REQ_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2696,9 +2810,14 @@ static int sample_conv_table_kbytes_in(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_IN_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_ull) >> 10;
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2732,9 +2851,14 @@ static int sample_conv_table_kbytes_out(const struct arg *arg_p, struct sample *
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_BYTES_OUT_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_ull) >> 10;
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2768,9 +2892,14 @@ static int sample_conv_table_server_id(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_SERVER_ID);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_sint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2804,9 +2933,14 @@ static int sample_conv_table_sess_cnt(const struct arg *arg_p, struct sample *sm
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_SESS_CNT);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = stktable_data_cast(ptr, std_t_uint);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }
@@ -2840,10 +2974,15 @@ static int sample_conv_table_sess_rate(const struct arg *arg_p, struct sample *s
                return 1;
 
        ptr = stktable_data_ptr(t, ts, STKTABLE_DT_SESS_RATE);
-       if (ptr)
+       if (ptr) {
+               HA_RWLOCK_RDLOCK(STK_SESS_LOCK, &ts->lock);
+
                smp->data.u.sint = read_freq_ctr_period(&stktable_data_cast(ptr, std_t_frqp),
                                                        t->data_arg[STKTABLE_DT_SESS_RATE].u);
 
+               HA_RWLOCK_RDUNLOCK(STK_SESS_LOCK, &ts->lock);
+       }
+
        stktable_release(t, ts);
        return !!ptr;
 }