diff --git a/internal/collector/mscluster/AGENTS.md b/internal/collector/mscluster/AGENTS.md index f0b7f299..0a9d567e 100644 --- a/internal/collector/mscluster/AGENTS.md +++ b/internal/collector/mscluster/AGENTS.md @@ -32,6 +32,12 @@ the documentation miss this. The parser validates only the structure and accepts zero padding. `TestDumpPropertyLists` logs real lists as hex when `WINDOWS_EXPORTER_TEST_CLUSAPI_DUMP` is set on a cluster node. +- Each `Cluster` owns one control buffer (`controlScratch`). Like the handle, + it belongs to the running call, so calls never share it and no `sync.Pool` + is needed. `ParseProperties` returns `Property.Data` slices into it: copy + values out (`copyResourceProperties`, `readCommonProperties`) before the + next control, and never keep a parsed list past that point. `get` zeroes + the bytes, so a reused buffer reads like a new one. - `clusapi.Open` reports an unconfigured cluster or a missing ClusAPI with `errors.ErrUnsupported`; the shared collector test treats that as an unsupported role on hosts where `mscluster` is not a required fixture. diff --git a/internal/headers/clusapi/clusapi.go b/internal/headers/clusapi/clusapi.go index 5d96dc85..fa69bee8 100644 --- a/internal/headers/clusapi/clusapi.go +++ b/internal/headers/clusapi/clusapi.go @@ -38,6 +38,7 @@ const ( resourceGetType = 0x0100002d resourceGetClassInfo = 0x0100000d maxBufferSize = 64 << 20 + maxScratchSize = 64 << 10 stateUnknown = ^uint32(0) // ClusterResourceStateUnknown propertyListBufferSize = 4 << 10 typeBufferSize = 512 @@ -74,7 +75,7 @@ type nativeAPI struct { enumName func(enum uintptr, index, enumType uint32, deadline time.Time) (objectName, error) openResource func(cluster uintptr, name objectName) (uintptr, error) closeResource func(resource uintptr) error - readResource func(resource uintptr, out *Resource, deadline time.Time) error + readResource func(resource uintptr, out *Resource, scratch *controlScratch, deadline time.Time) error } //nolint:gochecknoglobals @@ -158,6 +159,46 @@ type Cluster struct { handle uintptr closed bool running bool + + // scratch belongs to the running call, like handle. + scratch controlScratch +} + +// controlScratch is the reusable control buffer of a Cluster. Its owner runs +// one control at a time, so no pool is needed. Values parsed from a result, +// such as Property.Data, point into the buffer and are only valid until the +// next control; callers copy them out first. +type controlScratch struct { + buffer []byte +} + +// get returns n zeroed bytes, the same contents as a new buffer. +func (s *controlScratch) get(n int) []byte { + if cap(s.buffer) < n { + s.buffer = make([]byte, max(n, propertyListBufferSize)) + } + + buffer := s.buffer[:n] + clear(buffer) + + return buffer +} + +// keep holds on to a buffer a control had to grow, up to maxScratchSize, so +// the next object starts with the larger size. +func (s *controlScratch) keep(data []byte) { + if cap(data) > cap(s.buffer) && cap(data) <= maxScratchSize { + s.buffer = data[:cap(data)] + } +} + +// resourceControl calls ClusterResourceControl starting with n bytes of the +// scratch buffer. The result is only valid until the next control. +func (s *controlScratch) resourceControl(handle uintptr, code uint32, n int, deadline time.Time) ([]byte, error) { + data, err := resourceBufferIn(handle, code, s.get(n), deadline) + s.keep(data) + + return data, err } type Resource struct { @@ -391,7 +432,7 @@ func (c *Cluster) Resources(deadline time.Time) (_ []Resource, resultErr error) return fmt.Errorf("OpenClusterResourceEx: %w", err) } - err = api.readResource(handle, &resource, deadline) + err = api.readResource(handle, &resource, &c.scratch, deadline) if closeErr := api.closeResource(handle); closeErr != nil { err = errors.Join(err, fmt.Errorf("CloseClusterResource: %w", closeErr)) } @@ -459,13 +500,13 @@ func enumName(enum uintptr, index, enumType uint32, deadline time.Time) (objectN // readResource fills the properties of an open resource. Failed properties are // omitted; the identity is only valid once the state and owner are known. -func readResource(handle uintptr, resource *Resource, deadline time.Time) error { +func readResource(handle uintptr, resource *Resource, scratch *controlScratch, deadline time.Time) error { var resultErr error hasType := false for _, code := range []uint32{resourceGetROCommonProperties, resourceGetCommonProperties} { - data, err := resourceBuffer(handle, code, propertyListBufferSize, deadline) + data, err := scratch.resourceControl(handle, code, propertyListBufferSize, deadline) if err != nil { resultErr = errors.Join(resultErr, fmt.Errorf("ClusterResourceControl %#x: %w", code, err)) @@ -479,23 +520,8 @@ func readResource(handle uintptr, resource *Resource, deadline time.Time) error continue } - for name, values := range properties { - if len(values) != 1 { - continue - } - - if value, err := values[0].DWORD(); err == nil { - resource.Values[name] = value - } - } - - // The read-only common Type property is the same resource type name - // that CLUSCTL_RESOURCE_GET_RESOURCE_TYPE returns. - if values := properties["Type"]; len(values) == 1 { - if value, err := values[0].String(); err == nil { - resource.Type = value - hasType = true - } + if copyResourceProperties(resource, properties) { + hasType = true } } @@ -503,7 +529,7 @@ func readResource(handle uintptr, resource *Resource, deadline time.Time) error name string code uint32 }{{"Characteristics", resourceGetCharacteristics}, {"Flags", resourceGetFlags}} { - data, err := resourceBuffer(handle, field.code, 4, deadline) + data, err := scratch.resourceControl(handle, field.code, 4, deadline) switch { case err != nil: resultErr = errors.Join(resultErr, fmt.Errorf("%s: %w", field.name, err)) @@ -514,7 +540,7 @@ func readResource(handle uintptr, resource *Resource, deadline time.Time) error } } - data, err := resourceBuffer(handle, resourceGetClassInfo, 8, deadline) + data, err := scratch.resourceControl(handle, resourceGetClassInfo, 8, deadline) if err != nil { resultErr = errors.Join(resultErr, fmt.Errorf("class information: %w", err)) } else { @@ -523,7 +549,7 @@ func readResource(handle uintptr, resource *Resource, deadline time.Time) error // The property lists normally carry the type; only ask separately without it. if !hasType { - data, err = resourceBuffer(handle, resourceGetType, typeBufferSize, deadline) + data, err = scratch.resourceControl(handle, resourceGetType, typeBufferSize, deadline) if err != nil { return errors.Join(resultErr, fmt.Errorf("resource type: %w", err)) } @@ -544,6 +570,33 @@ func readResource(handle uintptr, resource *Resource, deadline time.Time) error return resultErr } +// copyResourceProperties copies the single DWORD values and the type name out +// of a parsed property list, so the list's buffer can be reused. It reports +// whether the list had the type. +func copyResourceProperties(resource *Resource, properties map[string][]Property) bool { + for name, values := range properties { + if len(values) != 1 { + continue + } + + if value, ok := values[0].dword(); ok { + resource.Values[name] = value + } + } + + // The read-only common Type property is the same resource type name that + // CLUSCTL_RESOURCE_GET_RESOURCE_TYPE returns. + if values := properties["Type"]; len(values) == 1 { + if value, err := values[0].String(); err == nil { + resource.Type = value + + return true + } + } + + return false +} + // parseResourceClassInfo decodes CLUS_RESOURCE_CLASS_INFO. Some resource DLLs, // such as the Storage Spaces Direct Health Service, succeed but return only // the class DWORD without the subclass; the WMI provider reports their @@ -563,8 +616,10 @@ func parseResourceClassInfo(data []byte) (uint32, uint32) { return class, subclass } -func resourceBuffer(handle uintptr, code uint32, size int, deadline time.Time) ([]byte, error) { - return controlBuffer(deadline, size, func(buffer []byte) (uint32, error) { +// resourceBufferIn calls ClusterResourceControl starting with buffer. The +// result may share buffer's memory. +func resourceBufferIn(handle uintptr, code uint32, buffer []byte, deadline time.Time) ([]byte, error) { + return controlBufferIn(deadline, buffer, func(buffer []byte) (uint32, error) { var ( size uint32 pointer *byte @@ -597,7 +652,8 @@ func readResourceState(handle uintptr, deadline time.Time) (uint32, string, stri } func stateBuffers(deadline time.Time, call func([]uint16, []uint16) (uint32, uint32, uint32, error)) (uint32, string, string, error) { - node, group := make([]uint16, 256), make([]uint16, 256) + units := make([]uint16, 512) + node, group := units[:256:256], units[256:] for range 16 { if err := checkDeadline(deadline); err != nil { @@ -655,15 +711,20 @@ func checkDeadline(deadline time.Time) error { return nil } -// controlBuffer starts with a buffer of the expected size, so most controls need -// one RPC. ERROR_MORE_DATA still grows the buffer to the reported size; size 0 -// starts with a NULL size probe. -func controlBuffer(deadline time.Time, size int, call func([]byte) (uint32, error)) ([]byte, error) { - var buffer []byte - if size > 0 { - buffer = make([]byte, size) +// newControlBuffer returns a buffer of the expected size, so most controls +// need one RPC; size 0 starts with a NULL size probe. +func newControlBuffer(size int) []byte { + if size <= 0 { + return nil } + return make([]byte, size) +} + +// controlBufferIn calls a control starting with buffer; an empty buffer is a +// NULL size probe. ERROR_MORE_DATA grows the buffer to the reported size, +// within buffer's capacity when it fits. The result may share buffer's memory. +func controlBufferIn(deadline time.Time, buffer []byte, call func([]byte) (uint32, error)) ([]byte, error) { for range 16 { if err := checkDeadline(deadline); err != nil { return nil, err @@ -675,7 +736,7 @@ func controlBuffer(deadline time.Time, size int, call func([]byte) (uint32, erro return nil, errors.New("invalid ClusterResourceControl buffer size") } - buffer = make([]byte, size) + buffer = growBuffer(buffer, int(size)) continue } @@ -689,7 +750,7 @@ func controlBuffer(deadline time.Time, size int, call func([]byte) (uint32, erro return nil, errors.New("invalid ClusterResourceControl buffer size") } - buffer = make([]byte, size) + buffer = growBuffer(buffer, int(size)) continue } @@ -703,3 +764,15 @@ func controlBuffer(deadline time.Time, size int, call func([]byte) (uint32, erro return nil, errors.New("ClusterResourceControl buffer did not stabilize") } + +// growBuffer returns size zeroed bytes, reusing buffer's capacity when it fits. +func growBuffer(buffer []byte, size int) []byte { + if size > cap(buffer) { + return make([]byte, size) + } + + buffer = buffer[:size] + clear(buffer) + + return buffer +} diff --git a/internal/headers/clusapi/clusapi_test.go b/internal/headers/clusapi/clusapi_test.go index 37fd4f03..a64359ae 100644 --- a/internal/headers/clusapi/clusapi_test.go +++ b/internal/headers/clusapi/clusapi_test.go @@ -29,7 +29,7 @@ import ( func TestControlBuffer(t *testing.T) { calls := 0 - data, err := controlBuffer(time.Time{}, 0, func(buffer []byte) (uint32, error) { + data, err := controlBufferIn(time.Time{}, newControlBuffer(0), func(buffer []byte) (uint32, error) { calls++ switch calls { case 1: @@ -62,7 +62,7 @@ func TestControlBuffer(t *testing.T) { func TestControlBufferPresized(t *testing.T) { calls := 0 - data, err := controlBuffer(time.Time{}, 8, func(buffer []byte) (uint32, error) { + data, err := controlBufferIn(time.Time{}, newControlBuffer(8), func(buffer []byte) (uint32, error) { calls++ if len(buffer) != 8 { @@ -79,7 +79,7 @@ func TestControlBufferPresized(t *testing.T) { calls = 0 - data, err = controlBuffer(time.Time{}, 4, func(buffer []byte) (uint32, error) { + data, err = controlBufferIn(time.Time{}, newControlBuffer(4), func(buffer []byte) (uint32, error) { calls++ if calls == 1 { return 6, windows.ERROR_MORE_DATA @@ -96,7 +96,7 @@ func TestControlBufferPresized(t *testing.T) { } // A pre-sized buffer that is already large enough must not be shrunk or retried. - _, err = controlBuffer(time.Time{}, 8, func([]byte) (uint32, error) { return 4, windows.ERROR_MORE_DATA }) + _, err = controlBufferIn(time.Time{}, newControlBuffer(8), func([]byte) (uint32, error) { return 4, windows.ERROR_MORE_DATA }) if err == nil { t.Fatal("accepted ERROR_MORE_DATA without growth") } @@ -114,7 +114,7 @@ func TestControlBufferInvalidSizes(t *testing.T) { {"unrelated_error", 0, windows.ERROR_ACCESS_DENIED}, } { t.Run(tc.name, func(t *testing.T) { - _, err := controlBuffer(time.Time{}, 0, func([]byte) (uint32, error) { return tc.size, tc.err }) + _, err := controlBufferIn(time.Time{}, newControlBuffer(0), func([]byte) (uint32, error) { return tc.size, tc.err }) if err == nil { t.Fatal("accepted invalid native response") } @@ -130,7 +130,7 @@ func TestControlBufferDeadline(t *testing.T) { calls := 0 deadline := time.Now().Add(time.Millisecond) - _, err := controlBuffer(deadline, 0, func([]byte) (uint32, error) { + _, err := controlBufferIn(deadline, newControlBuffer(0), func([]byte) (uint32, error) { calls++ time.Sleep(2 * time.Millisecond) @@ -141,7 +141,7 @@ func TestControlBufferDeadline(t *testing.T) { t.Fatalf("calls = %d, err = %v", calls, err) } - _, err = controlBuffer(time.Now().Add(-time.Second), 0, func([]byte) (uint32, error) { + _, err = controlBufferIn(time.Now().Add(-time.Second), newControlBuffer(0), func([]byte) (uint32, error) { t.Fatal("started native call after deadline") return 0, nil @@ -236,7 +236,7 @@ func TestStateBuffersInvalidNativeResponses(t *testing.T) { func TestControlBufferSuccessfulSizeProbe(t *testing.T) { calls := 0 - data, err := controlBuffer(time.Time{}, 0, func(buffer []byte) (uint32, error) { + data, err := controlBufferIn(time.Time{}, newControlBuffer(0), func(buffer []byte) (uint32, error) { calls++ if calls == 1 { return 4, nil diff --git a/internal/headers/clusapi/cluster.go b/internal/headers/clusapi/cluster.go index d1617501..9d5a4234 100644 --- a/internal/headers/clusapi/cluster.go +++ b/internal/headers/clusapi/cluster.go @@ -73,7 +73,7 @@ func (c *Cluster) Properties(deadline time.Time) (_ Object, resultErr error) { object := Object{Name: name, Values: make(map[string]uint32)} - if err := readCommonProperties(clusterControl, c.handle, objectCluster, object.Values, deadline); err != nil { + if err := readCommonProperties(clusterControl, c.handle, objectCluster, object.Values, &c.scratch, deadline); err != nil { resultErr = errors.Join(resultErr, err) if errors.Is(err, context.DeadlineExceeded) { @@ -157,7 +157,7 @@ func (c *Cluster) resourceClass(name string, deadline time.Time) (_ uint32, resu } }() - data, err := resourceBuffer(handle, resourceGetClassInfo, 8, deadline) + data, err := c.scratch.resourceControl(handle, resourceGetClassInfo, 8, deadline) if err != nil { return 0, fmt.Errorf("class information: %w", err) } diff --git a/internal/headers/clusapi/dump_test.go b/internal/headers/clusapi/dump_test.go index a3e5d792..e7cb994e 100644 --- a/internal/headers/clusapi/dump_test.go +++ b/internal/headers/clusapi/dump_test.go @@ -60,7 +60,7 @@ func TestDumpPropertyLists(t *testing.T) { deadline := time.Now().Add(time.Minute) for _, code := range []uint32{objectCluster<<24 | ctlGetROCommonProperties, objectCluster<<24 | ctlGetCommonProperties} { - data, err := controlBuffer(deadline, 0, func(buffer []byte) (uint32, error) { + data, err := controlBufferIn(deadline, newControlBuffer(0), func(buffer []byte) (uint32, error) { var ( size uint32 pointer *byte @@ -88,7 +88,7 @@ func TestDumpPropertyLists(t *testing.T) { } for _, code := range []uint32{resourceTypeGetROCommonProperties, resourceTypeGetCommonProperties, resourceTypeGetPrivateProperties} { - data, err := controlBuffer(deadline, 0, func(buffer []byte) (uint32, error) { + data, err := controlBufferIn(deadline, newControlBuffer(0), func(buffer []byte) (uint32, error) { var ( size uint32 pointer *byte @@ -133,11 +133,11 @@ func TestDumpPropertyLists(t *testing.T) { } for _, code := range []uint32{resourceGetROCommonProperties, resourceGetCommonProperties, resourceGetROPrivateProperties, resourceGetPrivateProperties} { - data, err := resourceBuffer(handle, code, 0, deadline) + data, err := resourceBufferIn(handle, code, nil, deadline) logPropertyList(t, fmt.Sprintf("resource %q %#x", name.name, code), data, err) } - data, err := resourceBuffer(handle, resourceGetType, 0, deadline) + data, err := resourceBufferIn(handle, resourceGetType, nil, deadline) t.Logf("resource %q GET_RESOURCE_TYPE: %s err=%v", name.name, hex.EncodeToString(data), err) closeResource.Call(handle) //nolint:errcheck diff --git a/internal/headers/clusapi/lifecycle_test.go b/internal/headers/clusapi/lifecycle_test.go index 0e1050b0..c0d17546 100644 --- a/internal/headers/clusapi/lifecycle_test.go +++ b/internal/headers/clusapi/lifecycle_test.go @@ -98,7 +98,7 @@ func (f *fakeAPI) api() *nativeAPI { return 2, nil }, closeResource: func(uintptr) error { return nil }, - readResource: func(_ uintptr, resource *Resource, _ time.Time) error { + readResource: func(_ uintptr, resource *Resource, _ *controlScratch, _ time.Time) error { resource.Values["State"] = 2 resource.IdentityValid = true diff --git a/internal/headers/clusapi/objects.go b/internal/headers/clusapi/objects.go index b4251d72..769b071b 100644 --- a/internal/headers/clusapi/objects.go +++ b/internal/headers/clusapi/objects.go @@ -210,7 +210,7 @@ func (c *Cluster) readObject(api *objectAPI, name objectName, deadline time.Time } }() - if err := readCommonProperties(api.control, handle, api.object, object.Values, deadline); err != nil { + if err := readCommonProperties(api.control, handle, api.object, object.Values, &c.scratch, deadline); err != nil { resultErr = errors.Join(resultErr, err) if errors.Is(err, context.DeadlineExceeded) { @@ -222,7 +222,7 @@ func (c *Cluster) readObject(api *objectAPI, name objectName, deadline time.Time name string operation uint32 }{{"Characteristics", ctlGetCharacteristics}, {"Flags", ctlGetFlags}} { - data, err := objectBuffer(api.control, handle, api.object<<24|field.operation, 4, deadline) + data, err := c.scratch.objectControl(api.control, handle, api.object<<24|field.operation, 4, deadline) switch { case err != nil: @@ -252,13 +252,13 @@ func (c *Cluster) readObject(api *objectAPI, name objectName, deadline time.Time // readCommonProperties stores the single 32-bit values of the read-only and // read/write common property lists in values. Other formats are skipped. -func readCommonProperties(control *windows.LazyProc, handle uintptr, object uint32, values map[string]uint32, deadline time.Time) error { +func readCommonProperties(control *windows.LazyProc, handle uintptr, object uint32, values map[string]uint32, scratch *controlScratch, deadline time.Time) error { var resultErr error for _, operation := range []uint32{ctlGetROCommonProperties, ctlGetCommonProperties} { code := object<<24 | operation - data, err := objectBuffer(control, handle, code, propertyListBufferSize, deadline) + data, err := scratch.objectControl(control, handle, code, propertyListBufferSize, deadline) if err != nil { resultErr = errors.Join(resultErr, fmt.Errorf("%s %#x: %w", control.Name, code, err)) @@ -281,7 +281,7 @@ func readCommonProperties(control *windows.LazyProc, handle uintptr, object uint continue } - if value, err := list[0].Value32(); err == nil { + if value, ok := list[0].value32(); ok { values[name] = value } } @@ -290,10 +290,21 @@ func readCommonProperties(control *windows.LazyProc, handle uintptr, object uint return resultErr } -// objectBuffer calls a Cluster{Group,Node,Network}Control function. They share -// the ClusterResourceControl signature and return a DWORD status code. -func objectBuffer(proc *windows.LazyProc, handle uintptr, code uint32, size int, deadline time.Time) ([]byte, error) { - return controlBuffer(deadline, size, func(buffer []byte) (uint32, error) { +// objectControl calls a Cluster{Group,Node,Network}Control function starting +// with n bytes of the scratch buffer. The result is only valid until the next +// control. +func (s *controlScratch) objectControl(proc *windows.LazyProc, handle uintptr, code uint32, n int, deadline time.Time) ([]byte, error) { + data, err := objectBufferIn(proc, handle, code, s.get(n), deadline) + s.keep(data) + + return data, err +} + +// objectBufferIn calls a Cluster{Group,Node,Network}Control function. They +// share the ClusterResourceControl signature and return a DWORD status code. +// The result may share buffer's memory. +func objectBufferIn(proc *windows.LazyProc, handle uintptr, code uint32, buffer []byte, deadline time.Time) ([]byte, error) { + return controlBufferIn(deadline, buffer, func(buffer []byte) (uint32, error) { var ( size uint32 pointer *byte diff --git a/internal/headers/clusapi/properties.go b/internal/headers/clusapi/properties.go index 5660215f..1fa146bb 100644 --- a/internal/headers/clusapi/properties.go +++ b/internal/headers/clusapi/properties.go @@ -19,6 +19,8 @@ import ( "encoding/binary" "errors" "fmt" + "strings" + "unicode" "unicode/utf16" ) @@ -40,11 +42,21 @@ type Property struct { } func (p Property) DWORD() (uint32, error) { - if p.Format != formatDWORD || len(p.Data) != 4 { + value, ok := p.dword() + if !ok { return 0, fmt.Errorf("expected DWORD, format %d, length %d", p.Format, len(p.Data)) } - return binary.LittleEndian.Uint32(p.Data), nil + return value, nil +} + +// dword is DWORD without the error, for callers that skip other formats. +func (p Property) dword() (uint32, bool) { + if p.Format != formatDWORD || len(p.Data) != 4 { + return 0, false + } + + return binary.LittleEndian.Uint32(p.Data), true } // String decodes an SZ, EXPAND_SZ or EXPANDED_SZ value like WMI does: the value @@ -61,11 +73,21 @@ func (p Property) String() (string, error) { // Value32 returns the bit pattern of a DWORD or LONG value. Callers decide the // signedness, matching the WMI type of the property they publish. func (p Property) Value32() (uint32, error) { - if (p.Format != formatDWORD && p.Format != formatLong) || len(p.Data) != 4 { + value, ok := p.value32() + if !ok { return 0, fmt.Errorf("expected DWORD or LONG, format %d, length %d", p.Format, len(p.Data)) } - return binary.LittleEndian.Uint32(p.Data), nil + return value, nil +} + +// value32 is Value32 without the error, for callers that skip other formats. +func (p Property) value32() (uint32, bool) { + if (p.Format != formatDWORD && p.Format != formatLong) || len(p.Data) != 4 { + return 0, false + } + + return binary.LittleEndian.Uint32(p.Data), true } // ParseProperties decodes CLUSPROP_LIST without casting untrusted bytes to native @@ -86,6 +108,9 @@ func ParseProperties(data []byte) (map[string][]Property, error) { } properties := make(map[string][]Property, count) + // Nearly every property has a single value, so all value lists share + // one backing array instead of one allocation per property. + all := make([]Property, 0, count) for range count { if len(data) < 8 || binary.LittleEndian.Uint32(data) != propertyName { @@ -99,7 +124,7 @@ func ParseProperties(data []byte) (map[string][]Property, error) { name := decodeString(nameBytes) data = rest - values := make([]Property, 0, 1) + start := len(all) for { if len(data) < 4 { @@ -127,13 +152,15 @@ func ParseProperties(data []byte) (map[string][]Property, error) { return nil, fmt.Errorf("invalid DWORD length for %q", name) } - values = append(values, Property{Format: format, Data: payload}) + all = append(all, Property{Format: format, Data: payload}) data = rest } // Keep the first occurrence; a repeated name does not affect the structure. + // The full slice expression keeps appends for later properties from + // writing into this list. if _, exists := properties[name]; !exists { - properties[name] = values + properties[name] = all[start:len(all):len(all)] } } @@ -165,16 +192,32 @@ func propertyPayload(data []byte) ([]byte, []byte, error) { // decodeString decodes little-endian UTF-16 bytes. A trailing odd byte is ignored. func decodeString(data []byte) string { - units := make([]uint16, len(data)/2) - for i := range units { - units[i] = binary.LittleEndian.Uint16(data[i*2:]) + // Names and type strings are short; converting them on the stack leaves + // the result as the only allocation. + var stack [128]uint16 + + units := stack[:0] + if len(data)/2 > len(stack) { + units = make([]uint16, 0, len(data)/2) + } + + // Stop at the first NUL: fixed-size fields such as MAX_PATH names would + // otherwise always outgrow the stack buffer. + for i := 0; i+1 < len(data); i += 2 { + unit := binary.LittleEndian.Uint16(data[i:]) + if unit == 0 { + break + } + + units = append(units, unit) } return decodeUnits(units) } // decodeUnits ends the string at the first NUL and replaces invalid UTF-16 with -// U+FFFD, matching the strings WMI returned for the same names. +// U+FFFD, matching the strings WMI returned for the same names. It decodes like +// utf16.Decode, but writes straight into the result instead of a []rune. func decodeUnits(units []uint16) string { for i, unit := range units { if unit == 0 { @@ -184,5 +227,28 @@ func decodeUnits(units []uint16) string { } } - return string(utf16.Decode(units)) + var builder strings.Builder + + builder.Grow(len(units)) + + for i := 0; i < len(units); i++ { + r := rune(units[i]) + + if utf16.IsSurrogate(r) { + decoded := unicode.ReplacementChar + if i+1 < len(units) { + decoded = utf16.DecodeRune(r, rune(units[i+1])) + } + + if decoded != unicode.ReplacementChar { + i++ + } + + r = decoded + } + + builder.WriteRune(r) + } + + return builder.String() } diff --git a/internal/headers/clusapi/properties_test.go b/internal/headers/clusapi/properties_test.go index 83828ef1..22b9f272 100644 --- a/internal/headers/clusapi/properties_test.go +++ b/internal/headers/clusapi/properties_test.go @@ -236,3 +236,128 @@ func BenchmarkParseProperties(b *testing.B) { } } } + +func encodeUTF16(units ...uint16) []byte { + data := make([]byte, 0, len(units)*2) + for _, unit := range units { + data = binary.LittleEndian.AppendUint16(data, unit) + } + + return data +} + +// referenceDecode is the utf16.Decode based decoding decodeString replaced. +func referenceDecode(data []byte) string { + units := make([]uint16, len(data)/2) + for i := range units { + units[i] = binary.LittleEndian.Uint16(data[i*2:]) + } + + for i, unit := range units { + if unit == 0 { + units = units[:i] + + break + } + } + + return string(utf16.Decode(units)) +} + +func TestDecodeString(t *testing.T) { + tests := map[string][]uint16{ + "empty": {}, + "ascii": utf16.Encode([]rune("Cluster IP Address")), + "nul": append(utf16.Encode([]rune("Type")), 0, 'x'), + "pair": utf16.Encode([]rune("disk 💾")), + "unpaired_high": {'a', 0xd83d, 'b'}, + "unpaired_low": {'a', 0xdcbe, 'b'}, + "high_before_nul": {'a', 0xd83d, 0, 'b'}, + "high_at_end": {'a', 0xd83d}, + "two_highs_then_low": {0xd83d, 0xd83d, 0xdcbe}, + "replacement_char_bmp": {0xfffd}, + } + for name, units := range tests { + t.Run(name, func(t *testing.T) { + data := encodeUTF16(units...) + if got, want := decodeString(data), referenceDecode(data); got != want { + t.Errorf("decodeString = %q, want %q", got, want) + } + + if got, want := decodeUnits(units), referenceDecode(data); got != want { + t.Errorf("decodeUnits = %q, want %q", got, want) + } + }) + } + + if got := decodeString([]byte{'a', 0, 'b'}); got != "a" { + t.Errorf("odd length = %q, want %q", got, "a") + } +} + +func FuzzDecodeString(f *testing.F) { + f.Add(encodeUTF16(utf16.Encode([]rune("Name 💾"))...)) + f.Add(encodeUTF16('a', 0xd83d, 'b')) + f.Fuzz(func(t *testing.T, data []byte) { + if got, want := decodeString(data), referenceDecode(data); got != want { + t.Fatalf("decodeString(% x) = %q, want %q", data, got, want) + } + }) +} + +func appendTestProperty(data []byte, name string, values ...uint32) []byte { + encoded := encodeUTF16(append(utf16.Encode([]rune(name)), 0)...) + + data = binary.LittleEndian.AppendUint32(data, propertyName) + data = binary.LittleEndian.AppendUint32(data, uint32(len(encoded))) + + data = append(data, encoded...) + for len(data)%4 != 0 { + data = append(data, 0) + } + + for _, value := range values { + data = binary.LittleEndian.AppendUint32(data, propertyValue|formatDWORD) + data = binary.LittleEndian.AppendUint32(data, 4) + data = binary.LittleEndian.AppendUint32(data, value) + } + + return binary.LittleEndian.AppendUint32(data, 0) +} + +// The value lists share one backing array; values of one property must not +// leak into another. +func TestParsePropertiesMultipleValues(t *testing.T) { + data := binary.LittleEndian.AppendUint32(nil, 3) + data = appendTestProperty(data, "First", 1, 2) + data = appendTestProperty(data, "Second", 3) + data = appendTestProperty(data, "Third", 4, 5, 6) + + properties, err := ParseProperties(data) + if err != nil { + t.Fatal(err) + } + + for name, want := range map[string][]uint32{"First": {1, 2}, "Second": {3}, "Third": {4, 5, 6}} { + values := properties[name] + if len(values) != len(want) { + t.Fatalf("%s: got %d values, want %d", name, len(values), len(want)) + } + + for i, value := range values { + if got, err := value.DWORD(); err != nil || got != want[i] { + t.Errorf("%s[%d] = %d, %v, want %d", name, i, got, err, want[i]) + } + } + } +} + +func BenchmarkDecodeString(b *testing.B) { + data := encodeUTF16(utf16.Encode([]rune("Microsoft Windows Server 2022 Datacenter"))...) + + b.ReportAllocs() + + for b.Loop() { + _ = decodeString(data) + } +} diff --git a/internal/headers/clusapi/scratch_test.go b/internal/headers/clusapi/scratch_test.go new file mode 100644 index 00000000..8d54f9d8 --- /dev/null +++ b/internal/headers/clusapi/scratch_test.go @@ -0,0 +1,186 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build windows + +package clusapi + +import ( + "bytes" + "maps" + "testing" + "time" + + "golang.org/x/sys/windows" +) + +// controlFromList answers a control with list, like ClusterResourceControl +// filling the caller's buffer. +func controlFromList(list []byte) func([]byte) (uint32, error) { + return func(buffer []byte) (uint32, error) { + if len(buffer) < len(list) { + return uint32(len(list)), windows.ERROR_MORE_DATA + } + + return uint32(copy(buffer, list)), nil + } +} + +// readListsIntoResource replays the property list part of readResource: both +// lists go through the same scratch buffer, one after the other. +func readListsIntoResource(tb testing.TB, scratch *controlScratch, resource *Resource, lists ...[]byte) bool { + tb.Helper() + + hasType := false + + for _, list := range lists { + data, err := controlBufferIn(time.Time{}, scratch.get(propertyListBufferSize), controlFromList(list)) + if err != nil { + tb.Fatal(err) + } + + scratch.keep(data) + + properties, err := ParseProperties(data) + if err != nil { + tb.Fatal(err) + } + + if copyResourceProperties(resource, properties) { + hasType = true + } + } + + return hasType +} + +func TestCopyResourceProperties(t *testing.T) { + var scratch controlScratch + + resource := Resource{Values: make(map[string]uint32)} + hasType := readListsIntoResource(t, &scratch, &resource, + readPropertyFixture(t, "resource_ro_common_generic_service.hex"), + readPropertyFixture(t, "resource_common_generic_service.hex"), + ) + + // The second list overwrote the buffer of the first; the values must have + // been copied out before. + want := map[string]uint32{ + "MonitorProcessId": 9092, + "DeadlockTimeout": 300000, + "EmbeddedFailureAction": 2, + "IsAlivePollInterval": ^uint32(0), + "LooksAlivePollInterval": ^uint32(0), + "PendingTimeout": 180000, + "PersistentState": 1, + "RestartAction": 2, + "RestartDelay": 500, + "RestartPeriod": 600000, + "RestartThreshold": 1, + "RetryPeriodOnFailure": 600000, + "SeparateMonitor": 0, + } + if !maps.Equal(resource.Values, want) { + t.Errorf("Values = %v, want %v", resource.Values, want) + } + + if !hasType || resource.Type != "Generic Service" { + t.Errorf("Type = %q, hasType = %t", resource.Type, hasType) + } + + clear(scratch.buffer) + + if resource.Type != "Generic Service" { + t.Errorf("Type changed with the buffer: %q", resource.Type) + } + + // The read-only list has no Type property. + resource = Resource{Values: make(map[string]uint32)} + if readListsIntoResource(t, &scratch, &resource, readPropertyFixture(t, "resource_ro_common_generic_service.hex")) { + t.Errorf("hasType for a list without Type, Type = %q", resource.Type) + } +} + +func TestControlScratch(t *testing.T) { + var scratch controlScratch + + buffer := scratch.get(8) + if len(buffer) != 8 || cap(buffer) != propertyListBufferSize { + t.Fatalf("get(8): len %d, cap %d", len(buffer), cap(buffer)) + } + + // A reused buffer must look like a new one: the cluster service may + // report more bytes than it wrote, and parsing relies on zero padding. + copy(buffer, "leftover") + + if buffer := scratch.get(8); !bytes.Equal(buffer, make([]byte, 8)) { + t.Errorf("get after use = % x, want zeros", buffer) + } + + // A buffer grown by ERROR_MORE_DATA is kept for the next object, up to + // maxScratchSize. + grown := make([]byte, 16<<10) + scratch.keep(grown[:100]) + + if cap(scratch.buffer) != len(grown) { + t.Errorf("keep: cap %d, want %d", cap(scratch.buffer), len(grown)) + } + + scratch.keep(make([]byte, maxScratchSize+1)) + + if cap(scratch.buffer) != len(grown) { + t.Errorf("kept a buffer above maxScratchSize: cap %d", cap(scratch.buffer)) + } +} + +func TestControlBufferInGrowsWithinCapacity(t *testing.T) { + var scratch controlScratch + + list := bytes.Repeat([]byte{1}, 1000) + start := scratch.get(typeBufferSize) + copy(start[:cap(start)], bytes.Repeat([]byte{0xff}, cap(start))) + + data, err := controlBufferIn(time.Time{}, start, controlFromList(list)) + if err != nil { + t.Fatal(err) + } + + if !bytes.Equal(data, list) { + t.Fatalf("data = % x", data[:16]) + } + + // ERROR_MORE_DATA reused the scratch buffer instead of allocating. + if &data[0] != &scratch.buffer[0] { + t.Error("grew into a new buffer although the capacity was enough") + } +} + +// BenchmarkReadResourceProperties replays the property list work readResource +// does for one resource with the captured CI lists. +func BenchmarkReadResourceProperties(b *testing.B) { + lists := [][]byte{ + readPropertyFixture(b, "resource_ro_common_generic_service.hex"), + readPropertyFixture(b, "resource_common_generic_service.hex"), + } + + var scratch controlScratch + + b.ReportAllocs() + + for b.Loop() { + resource := Resource{Values: make(map[string]uint32)} + readListsIntoResource(b, &scratch, &resource, lists...) + } +} diff --git a/internal/headers/clusapi/storage.go b/internal/headers/clusapi/storage.go index 6f6aa678..f5618e31 100644 --- a/internal/headers/clusapi/storage.go +++ b/internal/headers/clusapi/storage.go @@ -95,7 +95,7 @@ func (c *Cluster) readResourcePartitions(name objectName, deadline time.Time) (_ } }() - data, err := resourceBuffer(handle, resourceGetClassInfo, 8, deadline) + data, err := c.scratch.resourceControl(handle, resourceGetClassInfo, 8, deadline) if err != nil { return nil, fmt.Errorf("class information: %w", err) } @@ -104,7 +104,7 @@ func (c *Cluster) readResourcePartitions(name objectName, deadline time.Time) (_ return nil, nil } - data, err = resourceBuffer(handle, resourceStorageGetDiskInfoEx, propertyListBufferSize, deadline) + data, err = c.scratch.resourceControl(handle, resourceStorageGetDiskInfoEx, propertyListBufferSize, deadline) if errors.Is(err, windows.ERROR_INVALID_FUNCTION) || errors.Is(err, windows.ERROR_NOT_SUPPORTED) { // Storage class resources such as storage pools have no disk layout. return nil, nil