tree 5312862c83662583dcb45e86198711216f65bfcd
parent eca0ecfd2ee91828e73b35496c06c5e3b4ba7cfb
author Willy Tarreau <w@1wt.eu> 1591886255 +0200
committer William Lallemand <wlallemand@haproxy.org> 1591972665 +0200

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>