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.
This commit is contained in:
Willy Tarreau 2020-06-11 16:37:35 +02:00
parent 4989de2811
commit 2fc761e827

View file

@ -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;