From 0eecaa9547af380667a6e83cc3cebc224525e7f5 Mon Sep 17 00:00:00 2001 From: Derek Marcotte Date: Wed, 25 Oct 2017 14:55:22 -0400 Subject: [PATCH] Correct buffer_bytes > INT_MAX on BSD/amd64. (#712) * Correct buffer_bytes > INT_MAX on BSD/amd64. The sysctl vfs.bufspace returns either an int or a long, depending on the value. Large values of vfs.bufspace will result in error messages like: couldn't get meminfo: cannot allocate memory This will detect the returned data type, and cast appropriately. * Added explicit length checks per feedback. * Flatten Value() to make it easier to read. * Simplify per feedback. * Fix style. * Doc updates. --- collector/meminfo_bsd.go | 2 +- collector/sysctl_bsd.go | 109 ++++++++++++++++++++++++++------------- 2 files changed, 73 insertions(+), 38 deletions(-) diff --git a/collector/meminfo_bsd.go b/collector/meminfo_bsd.go index 0c722c24..d8eedb96 100644 --- a/collector/meminfo_bsd.go +++ b/collector/meminfo_bsd.go @@ -39,7 +39,7 @@ func (c *meminfoCollector) getMemInfo() (map[string]float64, error) { {name: "inactive_bytes", mib: "vm.stats.vm.v_inactive_count", conversion: fromPage}, {name: "wired_bytes", mib: "vm.stats.vm.v_wire_count", conversion: fromPage}, {name: "cache_bytes", mib: "vm.stats.vm.v_cache_count", conversion: fromPage}, - {name: "buffer_bytes", mib: "vfs.bufspace"}, + {name: "buffer_bytes", mib: "vfs.bufspace", dataType: bsdSysctlTypeCLong}, {name: "free_bytes", mib: "vm.stats.vm.v_free_count", conversion: fromPage}, {name: "size_bytes", mib: "vm.stats.vm.v_page_count", conversion: fromPage}, {name: "swap_in_bytes_total", mib: "vm.stats.vm.v_swappgsin", conversion: fromPage}, diff --git a/collector/sysctl_bsd.go b/collector/sysctl_bsd.go index 1aed5530..8fa54b43 100644 --- a/collector/sysctl_bsd.go +++ b/collector/sysctl_bsd.go @@ -35,9 +35,11 @@ const ( bsdSysctlTypeUint32 bsdSysctlType = iota bsdSysctlTypeUint64 bsdSysctlTypeStructTimeval + bsdSysctlTypeCLong ) -// Contains all the info needed to map a single bsd-sysctl to a prometheus value. +// Contains all the info needed to map a single bsd-sysctl to a prometheus +// value. type bsdSysctl struct { // Prometheus name name string @@ -72,42 +74,9 @@ func (b bsdSysctl) Value() (float64, error) { tmp64, err = unix.SysctlUint64(b.mib) tmpf64 = float64(tmp64) case bsdSysctlTypeStructTimeval: - raw, err := unix.SysctlRaw(b.mib) - if err != nil { - return 0, err - } - - /* - * From 10.3-RELEASE sources: - * - * /usr/include/sys/_timeval.h:47 - * time_t tv_sec - * suseconds_t tv_usec - * - * /usr/include/sys/_types.h:60 - * long __suseconds_t - * - * ... architecture dependent, via #ifdef: - * typedef __int64_t __time_t; - * typedef __int32_t __time_t; - */ - if len(raw) != (C.sizeof_time_t + C.sizeof_suseconds_t) { - // Shouldn't get here, unless the ABI changes... - return 0, fmt.Errorf( - "length of bytes received from sysctl (%d) does not match expected bytes (%d)", - len(raw), - C.sizeof_time_t+C.sizeof_suseconds_t, - ) - } - - secondsUp := unsafe.Pointer(&raw[0]) - susecondsUp := uintptr(secondsUp) + C.sizeof_time_t - unix := float64(*(*C.time_t)(secondsUp)) - usec := float64(*(*C.suseconds_t)(unsafe.Pointer(susecondsUp))) - - // This conversion maintains the usec precision. Using - // the time package did not. - tmpf64 = unix + (usec / float64(1000*1000)) + tmpf64, err = b.getStructTimeval() + case bsdSysctlTypeCLong: + tmpf64, err = b.getCLong() } if err != nil { @@ -120,3 +89,69 @@ func (b bsdSysctl) Value() (float64, error) { return tmpf64, nil } + +func (b bsdSysctl) getStructTimeval() (float64, error) { + raw, err := unix.SysctlRaw(b.mib) + if err != nil { + return 0, err + } + + /* + * From 10.3-RELEASE sources: + * + * /usr/include/sys/_timeval.h:47 + * time_t tv_sec + * suseconds_t tv_usec + * + * /usr/include/sys/_types.h:60 + * long __suseconds_t + * + * ... architecture dependent, via #ifdef: + * typedef __int64_t __time_t; + * typedef __int32_t __time_t; + */ + if len(raw) != (C.sizeof_time_t + C.sizeof_suseconds_t) { + // Shouldn't get here, unless the ABI changes... + return 0, fmt.Errorf( + "length of bytes received from sysctl (%d) does not match expected bytes (%d)", + len(raw), + C.sizeof_time_t+C.sizeof_suseconds_t, + ) + } + + secondsUp := unsafe.Pointer(&raw[0]) + susecondsUp := uintptr(secondsUp) + C.sizeof_time_t + unix := float64(*(*C.time_t)(secondsUp)) + usec := float64(*(*C.suseconds_t)(unsafe.Pointer(susecondsUp))) + + // This conversion maintains the usec precision. Using the time + // package did not. + return (unix + (usec / float64(1000*1000))), nil +} + +func (b bsdSysctl) getCLong() (float64, error) { + raw, err := unix.SysctlRaw(b.mib) + if err != nil { + return 0, err + } + + if len(raw) == C.sizeof_long { + return float64(*(*C.long)(unsafe.Pointer(&raw[0]))), nil + } + + if len(raw) == C.sizeof_int { + // This is valid for at least vfs.bufspace, and the default + // long handler - which can clamp longs to 32-bits: + // https://github.com/freebsd/freebsd/blob/releng/10.3/sys/kern/vfs_bio.c#L338 + // https://github.com/freebsd/freebsd/blob/releng/10.3/sys/kern/kern_sysctl.c#L1062 + return float64(*(*C.int)(unsafe.Pointer(&raw[0]))), nil + } + + return 0, fmt.Errorf( + "length of bytes received from sysctl (%d) does not match expected bytes (long: %d), (int: %d)", + len(raw), + C.sizeof_long, + C.sizeof_int, + ) + +}