BUG/MEDIUM: pattern: fix thread safety of pattern matching
Commit b5997f740 ("MAJOR: threads/map: Make acls/maps thread safe")
introduced a subtle bug in the pattern matching code. In order to cope
with the possibility that another thread might be modifying the pattern's
sample_data while it's being used, we return a thread-local static
sample_data which is a copy of the one found in the matched pattern. The
copy is performed depending on the sample_data's type. But the switch
statement misses some breaks and doesn't set the new sample_data pointer
at the right place, resulting in the original sample_data being restored
at the end before returning.
The net effect overall is that the correct sample_data is returned (hence
functionally speaking the matching works fine) but it's not thread-safe
so any del_map() or set_map() action could modify the pattern on one
thread while it's being used on another one. It doesn't seem likely to
cause a crash but could result in corrupted data appearing where the
value is consumed (e.g. when appended in a header or when logged) or an
ACL occasionally not matching after a map lookup.
This fix should be backported as far as 1.8.
Thanks to Tim for reporting it and to Emeric for the analysis.
(cherry picked from commit 2fc761e827071c4b976403e69836ff063994ee14)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 6ed341d13d3fef620f59e7dcdf41e17db0e01061)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
diff --git a/src/pattern.c b/src/pattern.c
index 743a309..a82900b 100644
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -2559,17 +2559,22 @@
if (static_sample_data.u.str.data >= static_sample_data.u.str.size)
static_sample_data.u.str.data = static_sample_data.u.str.size - 1;
memcpy(static_sample_data.u.str.area,
- pat->data->u.str.area,
- static_sample_data.u.str.data);
+ pat->data->u.str.area, static_sample_data.u.str.data);
static_sample_data.u.str.area[static_sample_data.u.str.data] = 0;
+ pat->data = &static_sample_data;
+ break;
+
case SMP_T_IPV4:
case SMP_T_IPV6:
case SMP_T_SINT:
memcpy(&static_sample_data, pat->data, sizeof(struct sample_data));
+ pat->data = &static_sample_data;
+ break;
default:
+ /* unimplemented pattern type */
pat->data = NULL;
+ break;
}
- pat->data = &static_sample_data;
}
HA_RWLOCK_RDUNLOCK(PATEXP_LOCK, &list->expr->lock);
return pat;