From: Aurelien DARRAGON Date: Tue, 14 Jan 2025 10:18:24 +0000 (+0100) Subject: BUG/MEDIUM: stktable: fix missing lock on some table converters X-Git-Tag: v3.2-dev4~34 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8919a80da9391e348aa325b44fdae40a35f48dcf;p=thirdparty%2Fhaproxy.git 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 122870059..852995a1a 100644 --- 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; }