Skip to content

Commit

Permalink
Correct buffer_bytes > INT_MAX on BSD/amd64. (#712)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
derekmarcotte authored and discordianfish committed Oct 25, 2017
1 parent 715ebd1 commit 0eecaa9
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 38 deletions.
2 changes: 1 addition & 1 deletion collector/meminfo_bsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
109 changes: 72 additions & 37 deletions collector/sysctl_bsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
)

}

0 comments on commit 0eecaa9

Please sign in to comment.