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.

diff --git a/src/pattern.c b/src/pattern.c
index 274e773ec45d54012253f088abc9e49f16dbbeb4..bd8b25a6185e7e26f25287069bcfc17a14cd1b32 100644 (file)
--- a/src/pattern.c
+++ b/src/pattern.c
@@ -2557,17 +2557,22 @@ struct pattern *pattern_exec_match(struct pattern_head *head, struct sample *smp
                                                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;