Fix memory corruption in name and unit labels

Signed-off-by: Piotr <17101802+thampiotr@users.noreply.github.com>
This commit is contained in:
Piotr 2025-07-31 13:57:27 +01:00
parent 3f96458782
commit 93bc3721fc
4 changed files with 42 additions and 14 deletions

View File

@ -787,7 +787,7 @@ func (b *ScratchBuilder) Add(name, value string) {
}
// Add a name/value pair, using []byte instead of string to reduce memory allocations.
// The values must remain live until Labels() is called.
// The values must remain live until Labels() or Overwrite() is called.
func (b *ScratchBuilder) UnsafeAddBytes(name, value []byte) {
b.add = append(b.add, Label{Name: yoloString(name), Value: yoloString(value)})
}
@ -830,6 +830,12 @@ func (b *ScratchBuilder) Overwrite(ls *Labels) {
ls.data = yoloString(b.overwriteBuffer)
}
// UnsafeString must be used when passing to Add strings that are reusing buffers via the unsafe package.
// For example, b.Add("__name__", labels.UnsafeString(yoloString(buf))).
func UnsafeString(s string) string {
return s // No-op - the call to Labels() or Overwrite() will create a copy
}
// SizeOfLabels returns the approximate space required for n copies of a label.
func SizeOfLabels(name, value string, n uint64) uint64 {
return uint64(len(name)+len(value)) + n*4 // Assuming most symbol-table entries are 2 bytes long.

View File

@ -506,6 +506,12 @@ func (b *ScratchBuilder) Overwrite(ls *Labels) {
*ls = append((*ls)[:0], b.add...)
}
// UnsafeString must be used when passing to Add strings that are reusing buffers via the unsafe package.
// For example, b.Add("__name__", labels.UnsafeString(yoloString(buf))).
func UnsafeString(s string) string {
return strings.Clone(s) // We need to clone such strings for slicelabels implementation.
}
// SizeOfLabels returns the approximate space required for n copies of a label.
func SizeOfLabels(name, value string, n uint64) uint64 {
return (uint64(len(name)) + uint64(unsafe.Sizeof(name)) + uint64(len(value)) + uint64(unsafe.Sizeof(value))) * n

View File

@ -619,7 +619,7 @@ func (b *ScratchBuilder) Add(name, value string) {
}
// UnsafeAddBytes adds a name/value pair using []byte instead of string to reduce memory allocations.
// The values must remain live until Labels() is called.
// The values must remain live until Labels() or Overwrite() is called.
func (b *ScratchBuilder) UnsafeAddBytes(name, value []byte) {
b.add = append(b.add, Label{Name: yoloString(name), Value: yoloString(value)})
}
@ -680,6 +680,12 @@ func (b *ScratchBuilder) SetSymbolTable(_ *SymbolTable) {
// no-op
}
// UnsafeString must be used when passing to Add strings that are reusing buffers via the unsafe package.
// For example, b.Add("__name__", labels.UnsafeString(yoloString(buf))).
func UnsafeString(s string) string {
return s // No-op - the call to Labels() or Overwrite() will create a copy
}
// SizeOfLabels returns the approximate space required for n copies of a label.
func SizeOfLabels(name, value string, n uint64) uint64 {
return uint64(labelSize(&Label{Name: name, Value: value})) * n

View File

@ -555,13 +555,19 @@ func (p *ProtobufParser) Next() (Entry, error) {
func (p *ProtobufParser) onSeriesOrHistogramUpdate() error {
p.builder.Reset()
name, safe := p.getMagicName()
if !safe {
name = labels.UnsafeString(name) // Make sure the name is safe to use in labels.
}
if p.enableTypeAndUnitLabels {
_, typ := p.Type()
m := schema.Metadata{
Name: p.getMagicName(),
Name: name,
Type: typ,
Unit: p.dec.GetUnit(),
// After the next call to dec.NextMetricFamily, the unit value becomes invalid, use labels.UnsafeString.
Unit: labels.UnsafeString(p.dec.GetUnit()),
}
m.AddToLabels(&p.builder)
if err := p.dec.Label(schema.IgnoreOverriddenMetadataLabelsScratchBuilder{
@ -571,7 +577,7 @@ func (p *ProtobufParser) onSeriesOrHistogramUpdate() error {
return err
}
} else {
p.builder.Add(labels.MetricName, p.getMagicName())
p.builder.Add(labels.MetricName, name)
if err := p.dec.Label(&p.builder); err != nil {
return err
}
@ -600,24 +606,28 @@ func (p *ProtobufParser) onSeriesOrHistogramUpdate() error {
return nil
}
// getMagicName usually just returns p.mf.GetType() but adds a magic suffix
// ("_count", "_sum", "_bucket") if needed according to the current parser
// state.
func (p *ProtobufParser) getMagicName() string {
// getMagicName returns the name of the metric. It usually just returns
// p.mf.GetName() but adds a magic suffix ("_count", "_sum", "_bucket") if
// needed according to the current parser state.
// The second return value is set to true if the metric name is safe to use
// in labels without wrapping with labels.UnsafeString. When it is false, the
// returned name will become invalid with the next call to dec.NextMetricFamily
// and must be wrapped with labels.UnsafeString when used in labels.
func (p *ProtobufParser) getMagicName() (string, bool) {
t := p.dec.GetType()
if p.state == EntryHistogram || (t != dto.MetricType_HISTOGRAM && t != dto.MetricType_GAUGE_HISTOGRAM && t != dto.MetricType_SUMMARY) {
return p.dec.GetName()
return p.dec.GetName(), false
}
if p.fieldPos == -2 {
return p.dec.GetName() + "_count"
return p.dec.GetName() + "_count", true
}
if p.fieldPos == -1 {
return p.dec.GetName() + "_sum"
return p.dec.GetName() + "_sum", true
}
if t == dto.MetricType_HISTOGRAM || t == dto.MetricType_GAUGE_HISTOGRAM {
return p.dec.GetName() + "_bucket"
return p.dec.GetName() + "_bucket", true
}
return p.dec.GetName()
return p.dec.GetName(), false
}
// getMagicLabel returns if a magic label ("quantile" or "le") is needed and, if