From ed324aad66177e523611873a10c5b9ad961a2863 Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 14 Oct 2022 09:49:53 +0300 Subject: [PATCH] lib/bytesutil: make sure that the string passed to FastStringMather.Match() is copied before using it as a key in the internal cache map This prevents from possible corruption of the internal cache map when the underlying byte slice used by the string key is modified. Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3227 --- docs/CHANGELOG.md | 2 ++ lib/bytesutil/fast_string_matcher.go | 6 ++++++ lib/bytesutil/fast_string_transformer.go | 2 ++ 3 files changed, 10 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 60cffcea5..30d7812c1 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -46,6 +46,8 @@ The following tip changes can be tested by building VictoriaMetrics components f * BUGFIX: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): set default region to `us-east-1` if `AWS_REGION` environment variable isn't set. The issue was introduced in [vmbackup v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820). See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3224). * BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager.html): fix deletion of old backups at [Azure blob storage](https://azure.microsoft.com/en-us/products/storage/blobs/). * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly merge buckets with identical `le` values, but with different string representation of these values when calculating [histogram_quantile](https://docs.victoriametrics.com/MetricsQL.html#histogram_quantile) and [histogram_share](https://docs.victoriametrics.com/MetricsQL.html#histogram_share). For example, `http_request_duration_seconds_bucket{le="5"}` and `http_requests_duration_seconds_bucket{le="5.0"}`. Such buckets may be returned from distinct targets. Thanks to @647-coder for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/3225). +* BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): properly apply regex filters when searching for time series. Previously unexpected time series could be returned from regex filter. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3227). The issue was introduced in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820). +* BUGFIX: [vmagent](https://docs.victoriametrics.com/vmbagent.html): properly apply `if` section with regex filters. Previously unexpected metrics could be returned from `if` section. The issue was introduced in [v1.82.0](https://docs.victoriametrics.com/CHANGELOG.html#v1820). ## [v1.82.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.82.0) diff --git a/lib/bytesutil/fast_string_matcher.go b/lib/bytesutil/fast_string_matcher.go index 40658e499..fdbc2b7e2 100644 --- a/lib/bytesutil/fast_string_matcher.go +++ b/lib/bytesutil/fast_string_matcher.go @@ -1,6 +1,7 @@ package bytesutil import ( + "strings" "sync" "sync/atomic" ) @@ -38,6 +39,11 @@ func (fsm *FastStringMatcher) Match(s string) bool { // Slow path - run matchFunc for s and store the result in the cache. b := fsm.matchFunc(s) bp := &b + // Make a copy of s in order to limit memory usage to the s length, + // since the s may point to bigger string. + // This also protects from the case when s contains unsafe string, which points to a temporary byte slice. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3227 + s = strings.Clone(s) m.Store(s, bp) n := atomic.AddUint64(&fsm.mLen, 1) if n > 100e3 { diff --git a/lib/bytesutil/fast_string_transformer.go b/lib/bytesutil/fast_string_transformer.go index 60549f6eb..2dfa31af0 100644 --- a/lib/bytesutil/fast_string_transformer.go +++ b/lib/bytesutil/fast_string_transformer.go @@ -40,6 +40,8 @@ func (fst *FastStringTransformer) Transform(s string) string { sTransformed := fst.transformFunc(s) // Make a copy of s in order to limit memory usage to the s length, // since the s may point to bigger string. + // This also protects from the case when s contains unsafe string, which points to a temporary byte slice. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3227 s = strings.Clone(s) if sTransformed == s { // point sTransformed to just allocated s, since it may point to s,