diff --git a/CHANGELOG.md b/CHANGELOG.md index 15f5c83bff1..94a44febb76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ It is recommend to upgrade the storage components first (Receive, Store, etc.) a ### Fixed - [#8128](https://github.com/thanos-io/thanos/issues/8128): Query-Frontend: Fix panic in `AnalyzesMerge` caused by indexing the wrong slice variable, leading to an out-of-range access when merging more than two query analyses. +- [#8709](https://github.com/thanos-io/thanos/pull/8709): Query/Query-Frontend: fix panic in `ZLabelsToPromLabels` and `FromLabelAdaptersToLabels` caused by unsafe pointer casts incompatible with Prometheus `stringlabels` encoding. ### Added diff --git a/internal/cortex/cortexpb/compat.go b/internal/cortex/cortexpb/compat.go index 48d9c3889ca..78a561563ab 100644 --- a/internal/cortex/cortexpb/compat.go +++ b/internal/cortex/cortexpb/compat.go @@ -19,21 +19,29 @@ import ( "github.com/thanos-io/thanos/internal/cortex/util" ) -// FromLabelAdaptersToLabels casts []LabelAdapter to labels.Labels. -// It uses unsafe, but as LabelAdapter == labels.Label this should be safe. -// This allows us to use labels.Labels directly in protos. -// -// Note: while resulting labels.Labels is supposedly sorted, this function -// doesn't enforce that. If input is not sorted, output will be wrong. +// FromLabelAdaptersToLabels converts []LabelAdapter to labels.Labels. func FromLabelAdaptersToLabels(ls []LabelAdapter) labels.Labels { - return *(*labels.Labels)(unsafe.Pointer(&ls)) + if len(ls) == 0 { + return labels.EmptyLabels() + } + b := labels.NewScratchBuilder(len(ls)) + for _, l := range ls { + b.Add(l.Name, l.Value) + } + b.Sort() + return b.Labels() } -// FromLabelsToLabelAdapters casts labels.Labels to []LabelAdapter. -// It uses unsafe, but as LabelAdapter == labels.Label this should be safe. -// This allows us to use labels.Labels directly in protos. +// FromLabelsToLabelAdapters converts labels.Labels to []LabelAdapter. func FromLabelsToLabelAdapters(ls labels.Labels) []LabelAdapter { - return *(*[]LabelAdapter)(unsafe.Pointer(&ls)) + if ls.IsEmpty() { + return nil + } + result := make([]LabelAdapter, 0, ls.Len()) + ls.Range(func(l labels.Label) { + result = append(result, LabelAdapter{Name: l.Name, Value: l.Value}) + }) + return result } // FromLabelAdaptersToMetric converts []LabelAdapter to a model.Metric. diff --git a/pkg/store/labelpb/label.go b/pkg/store/labelpb/label.go index 38bcc63421f..98a1bab1a4c 100644 --- a/pkg/store/labelpb/label.go +++ b/pkg/store/labelpb/label.go @@ -38,18 +38,25 @@ func noAllocBytes(buf string) []byte { return *(*[]byte)(unsafe.Pointer(&buf)) } -// ZLabelsFromPromLabels converts Prometheus labels to slice of labelpb.ZLabel in type unsafe manner. -// It reuses the same memory. Caller should abort using passed labels.Labels. +// ZLabelsFromPromLabels converts Prometheus labels to slice of labelpb.ZLabel. func ZLabelsFromPromLabels(lset labels.Labels) []ZLabel { - return *(*[]ZLabel)(unsafe.Pointer(&lset)) + if lset.IsEmpty() { + return nil + } + zlabels := make([]ZLabel, 0, lset.Len()) + lset.Range(func(l labels.Label) { + zlabels = append(zlabels, ZLabel{Name: l.Name, Value: l.Value}) + }) + return zlabels } -// ZLabelsToPromLabels convert slice of labelpb.ZLabel to Prometheus labels in type unsafe manner. -// It reuses the same memory. Caller should abort using passed []ZLabel. -// NOTE: Use with care. ZLabels holds memory from the whole protobuf unmarshal, so the returned -// Prometheus Labels will hold this memory as well. +// ZLabelsToPromLabels converts slice of labelpb.ZLabel to Prometheus labels. func ZLabelsToPromLabels(lset []ZLabel) labels.Labels { - return *(*labels.Labels)(unsafe.Pointer(&lset)) + b := labels.NewScratchBuilder(len(lset)) + for _, l := range lset { + b.Add(l.Name, l.Value) + } + return b.Labels() } // ReAllocAndInternZLabelsStrings re-allocates all underlying bytes for string, detaching it from bigger memory pool. diff --git a/pkg/store/labelpb/label_zlabel_test.go b/pkg/store/labelpb/label_zlabel_test.go index 6d7d68debdf..c1253d2d431 100644 --- a/pkg/store/labelpb/label_zlabel_test.go +++ b/pkg/store/labelpb/label_zlabel_test.go @@ -38,10 +38,9 @@ func TestLabelsToPromLabels_LabelsToPromLabels(t *testing.T) { zlset[i].Value += "yolo" } + // ZLabelsFromPromLabels now copies data, so modifying zlset does not affect the original. m := lset.Map() - testutil.Equals(t, "1yolo", m["a"]) - - m["a"] = "1" + testutil.Equals(t, "1", m["a"]) testutil.Equals(t, testLsetMap, m) }