Browse Source

collector/zfs: Prevent `procfs` integer underflow (#2961)

* collector/zfs: Prevent `procfs` integer underflow

Prevent integer underflow when parsing the `procfs` file as it used a
`ParseUint` to parse signed values.

Fixes: #2766
---------

Signed-off-by: Pranshu Srivastava <rexagod@gmail.com>
pull/3073/head
Pranshu Srivastava 4 months ago committed by GitHub
parent
commit
ff20a6ac48
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 3
      collector/fixtures/e2e-64k-page-output.txt
  2. 3
      collector/fixtures/e2e-output.txt
  3. 143
      collector/fixtures/proc/spl/kstat/zfs/arcstats
  4. 4
      collector/zfs.go
  5. 24
      collector/zfs_linux.go
  6. 59
      collector/zfs_linux_test.go

3
collector/fixtures/e2e-64k-page-output.txt

@ -3776,6 +3776,9 @@ node_zfs_arc_l2_writes_lock_retry 0
# HELP node_zfs_arc_l2_writes_sent kstat.zfs.misc.arcstats.l2_writes_sent # HELP node_zfs_arc_l2_writes_sent kstat.zfs.misc.arcstats.l2_writes_sent
# TYPE node_zfs_arc_l2_writes_sent untyped # TYPE node_zfs_arc_l2_writes_sent untyped
node_zfs_arc_l2_writes_sent 0 node_zfs_arc_l2_writes_sent 0
# HELP node_zfs_arc_memory_available_bytes kstat.zfs.misc.arcstats.memory_available_bytes
# TYPE node_zfs_arc_memory_available_bytes untyped
node_zfs_arc_memory_available_bytes -9.223372036854776e+17
# HELP node_zfs_arc_memory_direct_count kstat.zfs.misc.arcstats.memory_direct_count # HELP node_zfs_arc_memory_direct_count kstat.zfs.misc.arcstats.memory_direct_count
# TYPE node_zfs_arc_memory_direct_count untyped # TYPE node_zfs_arc_memory_direct_count untyped
node_zfs_arc_memory_direct_count 542 node_zfs_arc_memory_direct_count 542

3
collector/fixtures/e2e-output.txt

@ -3798,6 +3798,9 @@ node_zfs_arc_l2_writes_lock_retry 0
# HELP node_zfs_arc_l2_writes_sent kstat.zfs.misc.arcstats.l2_writes_sent # HELP node_zfs_arc_l2_writes_sent kstat.zfs.misc.arcstats.l2_writes_sent
# TYPE node_zfs_arc_l2_writes_sent untyped # TYPE node_zfs_arc_l2_writes_sent untyped
node_zfs_arc_l2_writes_sent 0 node_zfs_arc_l2_writes_sent 0
# HELP node_zfs_arc_memory_available_bytes kstat.zfs.misc.arcstats.memory_available_bytes
# TYPE node_zfs_arc_memory_available_bytes untyped
node_zfs_arc_memory_available_bytes -9.223372036854776e+17
# HELP node_zfs_arc_memory_direct_count kstat.zfs.misc.arcstats.memory_direct_count # HELP node_zfs_arc_memory_direct_count kstat.zfs.misc.arcstats.memory_direct_count
# TYPE node_zfs_arc_memory_direct_count untyped # TYPE node_zfs_arc_memory_direct_count untyped
node_zfs_arc_memory_direct_count 542 node_zfs_arc_memory_direct_count 542

143
collector/fixtures/proc/spl/kstat/zfs/arcstats

@ -1,93 +1,94 @@
6 1 0x01 91 4368 5266997922 97951858082072 6 1 0x01 91 4368 5266997922 97951858082072
name type data name type data
hits 4 8772612 anon_evictable_data 4 0
misses 4 604635 anon_evictable_metadata 4 0
anon_size 4 1917440
arc_loaned_bytes 4 0
arc_meta_limit 4 6275982336
arc_meta_max 4 449286096
arc_meta_min 4 16777216
arc_meta_used 4 308103632
arc_need_free 4 0
arc_no_grow 4 0
arc_prune 4 0
arc_sys_free 4 261496832
arc_tempreserve 4 0
c 4 1643208777
c_max 4 8367976448
c_min 4 33554432
data_size 4 1295836160
deleted 4 60403
demand_data_hits 4 7221032 demand_data_hits 4 7221032
demand_data_misses 4 73300 demand_data_misses 4 73300
demand_metadata_hits 4 1464353 demand_metadata_hits 4 1464353
demand_metadata_misses 4 498170 demand_metadata_misses 4 498170
prefetch_data_hits 4 3615 duplicate_buffers 4 0
prefetch_data_misses 4 17094 duplicate_buffers_size 4 0
prefetch_metadata_hits 4 83612 duplicate_reads 4 0
prefetch_metadata_misses 4 16071
mru_hits 4 855535
mru_ghost_hits 4 21100
mfu_hits 4 7829854
mfu_ghost_hits 4 821
deleted 4 60403
mutex_miss 4 2
evict_skip 4 2265729
evict_not_enough 4 680
evict_l2_cached 4 0 evict_l2_cached 4 0
evict_l2_eligible 4 8992514560 evict_l2_eligible 4 8992514560
evict_l2_ineligible 4 992552448 evict_l2_ineligible 4 992552448
evict_l2_skip 4 0 evict_l2_skip 4 0
evict_not_enough 4 680
evict_skip 4 2265729
hash_chain_max 4 3
hash_chains 4 412
hash_collisions 4 50564
hash_elements 4 42359 hash_elements 4 42359
hash_elements_max 4 88245 hash_elements_max 4 88245
hash_collisions 4 50564
hash_chains 4 412
hash_chain_max 4 3
p 4 516395305
c 4 1643208777
c_min 4 33554432
c_max 4 8367976448
size 4 1603939792
hdr_size 4 16361080 hdr_size 4 16361080
data_size 4 1295836160 hits 4 8772612
metadata_size 4 175298560 l2_abort_lowmem 4 0
other_size 4 116443992 l2_asize 4 0
anon_size 4 1917440 l2_cdata_free_on_write 4 0
anon_evictable_data 4 0 l2_cksum_bad 4 0
anon_evictable_metadata 4 0 l2_compress_failures 4 0
mru_size 4 402593792 l2_compress_successes 4 0
mru_evictable_data 4 278091264 l2_compress_zeros 4 0
mru_evictable_metadata 4 18606592 l2_evict_l1cached 4 0
mru_ghost_size 4 999728128 l2_evict_lock_retry 4 0
mru_ghost_evictable_data 4 883765248 l2_evict_reading 4 0
mru_ghost_evictable_metadata 4 115962880 l2_feeds 4 0
mfu_size 4 1066623488 l2_free_on_write 4 0
mfu_evictable_data 4 1017613824 l2_hdr_size 4 0
mfu_evictable_metadata 4 9163776
mfu_ghost_size 4 104936448
mfu_ghost_evictable_data 4 96731136
mfu_ghost_evictable_metadata 4 8205312
l2_hits 4 0 l2_hits 4 0
l2_io_error 4 0
l2_misses 4 0 l2_misses 4 0
l2_feeds 4 0
l2_rw_clash 4 0
l2_read_bytes 4 0 l2_read_bytes 4 0
l2_rw_clash 4 0
l2_size 4 0
l2_write_bytes 4 0 l2_write_bytes 4 0
l2_writes_sent 4 0
l2_writes_done 4 0 l2_writes_done 4 0
l2_writes_error 4 0 l2_writes_error 4 0
l2_writes_lock_retry 4 0 l2_writes_lock_retry 4 0
l2_evict_lock_retry 4 0 l2_writes_sent 4 0
l2_evict_reading 4 0 memory_available_bytes 3 -922337203685477580
l2_evict_l1cached 4 0
l2_free_on_write 4 0
l2_cdata_free_on_write 4 0
l2_abort_lowmem 4 0
l2_cksum_bad 4 0
l2_io_error 4 0
l2_size 4 0
l2_asize 4 0
l2_hdr_size 4 0
l2_compress_successes 4 0
l2_compress_zeros 4 0
l2_compress_failures 4 0
memory_throttle_count 4 0
duplicate_buffers 4 0
duplicate_buffers_size 4 0
duplicate_reads 4 0
memory_direct_count 4 542 memory_direct_count 4 542
memory_indirect_count 4 3006 memory_indirect_count 4 3006
arc_no_grow 4 0 memory_throttle_count 4 0
arc_tempreserve 4 0 metadata_size 4 175298560
arc_loaned_bytes 4 0 mfu_evictable_data 4 1017613824
arc_prune 4 0 mfu_evictable_metadata 4 9163776
arc_meta_used 4 308103632 mfu_ghost_evictable_data 4 96731136
arc_meta_limit 4 6275982336 mfu_ghost_evictable_metadata 4 8205312
arc_meta_max 4 449286096 mfu_ghost_hits 4 821
arc_meta_min 4 16777216 mfu_ghost_size 4 104936448
arc_need_free 4 0 mfu_hits 4 7829854
arc_sys_free 4 261496832 mfu_size 4 1066623488
misses 4 604635
mru_evictable_data 4 278091264
mru_evictable_metadata 4 18606592
mru_ghost_evictable_data 4 883765248
mru_ghost_evictable_metadata 4 115962880
mru_ghost_hits 4 21100
mru_ghost_size 4 999728128
mru_hits 4 855535
mru_size 4 402593792
mutex_miss 4 2
other_size 4 116443992
p 4 516395305
prefetch_data_hits 4 3615
prefetch_data_misses 4 17094
prefetch_metadata_hits 4 83612
prefetch_metadata_misses 4 16071
size 4 1603939792

4
collector/zfs.go

@ -95,7 +95,7 @@ func (s zfsSysctl) metricName() string {
return strings.Replace(parts[len(parts)-1], "-", "_", -1) return strings.Replace(parts[len(parts)-1], "-", "_", -1)
} }
func (c *zfsCollector) constSysctlMetric(subsystem string, sysctl zfsSysctl, value uint64) prometheus.Metric { func (c *zfsCollector) constSysctlMetric(subsystem string, sysctl zfsSysctl, value float64) prometheus.Metric {
metricName := sysctl.metricName() metricName := sysctl.metricName()
return prometheus.MustNewConstMetric( return prometheus.MustNewConstMetric(
@ -106,7 +106,7 @@ func (c *zfsCollector) constSysctlMetric(subsystem string, sysctl zfsSysctl, val
nil, nil,
), ),
prometheus.UntypedValue, prometheus.UntypedValue,
float64(value), value,
) )
} }

24
collector/zfs_linux.go

@ -63,8 +63,15 @@ func (c *zfsCollector) updateZfsStats(subsystem string, ch chan<- prometheus.Met
} }
defer file.Close() defer file.Close()
return c.parseProcfsFile(file, c.linuxPathMap[subsystem], func(s zfsSysctl, v uint64) { return c.parseProcfsFile(file, c.linuxPathMap[subsystem], func(s zfsSysctl, v interface{}) {
ch <- c.constSysctlMetric(subsystem, s, v) var valueAsFloat64 float64
switch value := v.(type) {
case int64:
valueAsFloat64 = float64(value)
case uint64:
valueAsFloat64 = float64(value)
}
ch <- c.constSysctlMetric(subsystem, s, valueAsFloat64)
}) })
} }
@ -144,7 +151,7 @@ func (c *zfsCollector) updatePoolStats(ch chan<- prometheus.Metric) error {
return nil return nil
} }
func (c *zfsCollector) parseProcfsFile(reader io.Reader, fmtExt string, handler func(zfsSysctl, uint64)) error { func (c *zfsCollector) parseProcfsFile(reader io.Reader, fmtExt string, handler func(zfsSysctl, interface{})) error {
scanner := bufio.NewScanner(reader) scanner := bufio.NewScanner(reader)
parseLine := false parseLine := false
@ -163,11 +170,18 @@ func (c *zfsCollector) parseProcfsFile(reader io.Reader, fmtExt string, handler
// kstat data type (column 2) should be KSTAT_DATA_UINT64, otherwise ignore // kstat data type (column 2) should be KSTAT_DATA_UINT64, otherwise ignore
// TODO: when other KSTAT_DATA_* types arrive, much of this will need to be restructured // TODO: when other KSTAT_DATA_* types arrive, much of this will need to be restructured
if parts[1] == kstatDataUint64 || parts[1] == kstatDataInt64 {
key := fmt.Sprintf("kstat.zfs.misc.%s.%s", fmtExt, parts[0]) key := fmt.Sprintf("kstat.zfs.misc.%s.%s", fmtExt, parts[0])
switch parts[1] {
case kstatDataUint64:
value, err := strconv.ParseUint(parts[2], 10, 64) value, err := strconv.ParseUint(parts[2], 10, 64)
if err != nil { if err != nil {
return fmt.Errorf("could not parse expected integer value for %q", key) return fmt.Errorf("could not parse expected unsigned integer value for %q: %w", key, err)
}
handler(zfsSysctl(key), value)
case kstatDataInt64:
value, err := strconv.ParseInt(parts[2], 10, 64)
if err != nil {
return fmt.Errorf("could not parse expected signed integer value for %q: %w", key, err)
} }
handler(zfsSysctl(key), value) handler(zfsSysctl(key), value)
} }

59
collector/zfs_linux_test.go

@ -35,24 +35,25 @@ func TestArcstatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(arcstatsFile, "arcstats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(arcstatsFile, "arcstats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.arcstats.hits") { if s == zfsSysctl("kstat.zfs.misc.arcstats.hits") {
return if v.(uint64) != uint64(8772612) {
t.Fatalf("Incorrect value parsed from procfs data")
} }
} else if s == zfsSysctl("kstat.zfs.misc.arcstats.memory_available_bytes") {
handlerCalled = true if v.(int64) != int64(-922337203685477580) {
if v != uint64(8772612) {
t.Fatalf("Incorrect value parsed from procfs data") t.Fatalf("Incorrect value parsed from procfs data")
} }
} else {
return
}
handlerCalled = true
}) })
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
if !handlerCalled { if !handlerCalled {
t.Fatal("Arcstats parsing handler was not called for some expected sysctls") t.Fatal("Arcstats parsing handler was not called for some expected sysctls")
} }
@ -71,7 +72,7 @@ func TestZfetchstatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(zfetchstatsFile, "zfetchstats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(zfetchstatsFile, "zfetchstats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.zfetchstats.hits") { if s != zfsSysctl("kstat.zfs.misc.zfetchstats.hits") {
return return
@ -79,7 +80,7 @@ func TestZfetchstatsParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(7067992) { if v.(uint64) != uint64(7067992) {
t.Fatalf("Incorrect value parsed from procfs data") t.Fatalf("Incorrect value parsed from procfs data")
} }
@ -107,7 +108,7 @@ func TestZilParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(zilFile, "zil", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(zilFile, "zil", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.zil.zil_commit_count") { if s != zfsSysctl("kstat.zfs.misc.zil.zil_commit_count") {
return return
@ -115,7 +116,7 @@ func TestZilParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(10) { if v.(uint64) != uint64(10) {
t.Fatalf("Incorrect value parsed from procfs data") t.Fatalf("Incorrect value parsed from procfs data")
} }
@ -143,7 +144,7 @@ func TestVdevCacheStatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(vdevCacheStatsFile, "vdev_cache_stats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(vdevCacheStatsFile, "vdev_cache_stats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.vdev_cache_stats.delegations") { if s != zfsSysctl("kstat.zfs.misc.vdev_cache_stats.delegations") {
return return
@ -151,7 +152,7 @@ func TestVdevCacheStatsParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(40) { if v.(uint64) != uint64(40) {
t.Fatalf("Incorrect value parsed from procfs data") t.Fatalf("Incorrect value parsed from procfs data")
} }
@ -179,7 +180,7 @@ func TestXuioStatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(xuioStatsFile, "xuio_stats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(xuioStatsFile, "xuio_stats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.xuio_stats.onloan_read_buf") { if s != zfsSysctl("kstat.zfs.misc.xuio_stats.onloan_read_buf") {
return return
@ -187,7 +188,7 @@ func TestXuioStatsParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(32) { if v.(uint64) != uint64(32) {
t.Fatalf("Incorrect value parsed from procfs data") t.Fatalf("Incorrect value parsed from procfs data")
} }
@ -215,7 +216,7 @@ func TestFmParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(fmFile, "fm", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(fmFile, "fm", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.fm.erpt-dropped") { if s != zfsSysctl("kstat.zfs.misc.fm.erpt-dropped") {
return return
@ -223,7 +224,7 @@ func TestFmParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(18) { if v.(uint64) != uint64(18) {
t.Fatalf("Incorrect value parsed from procfs data") t.Fatalf("Incorrect value parsed from procfs data")
} }
@ -251,7 +252,7 @@ func TestDmuTxParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(dmuTxFile, "dmu_tx", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(dmuTxFile, "dmu_tx", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.dmu_tx.dmu_tx_assigned") { if s != zfsSysctl("kstat.zfs.misc.dmu_tx.dmu_tx_assigned") {
return return
@ -259,7 +260,7 @@ func TestDmuTxParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(3532844) { if v.(uint64) != uint64(3532844) {
t.Fatalf("Incorrect value parsed from procfs data") t.Fatalf("Incorrect value parsed from procfs data")
} }
@ -367,7 +368,7 @@ func TestAbdstatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(abdstatsFile, "abdstats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(abdstatsFile, "abdstats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.abdstats.linear_data_size") { if s != zfsSysctl("kstat.zfs.misc.abdstats.linear_data_size") {
return return
@ -375,7 +376,7 @@ func TestAbdstatsParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(223232) { if v.(uint64) != uint64(223232) {
t.Fatalf("Incorrect value parsed from procfs abdstats data") t.Fatalf("Incorrect value parsed from procfs abdstats data")
} }
@ -403,7 +404,7 @@ func TestDbufstatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(dbufstatsFile, "dbufstats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(dbufstatsFile, "dbufstats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.dbufstats.hash_hits") { if s != zfsSysctl("kstat.zfs.misc.dbufstats.hash_hits") {
return return
@ -411,7 +412,7 @@ func TestDbufstatsParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(108807) { if v.(uint64) != uint64(108807) {
t.Fatalf("Incorrect value parsed from procfs dbufstats data") t.Fatalf("Incorrect value parsed from procfs dbufstats data")
} }
@ -439,7 +440,7 @@ func TestDnodestatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(dnodestatsFile, "dnodestats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(dnodestatsFile, "dnodestats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.dnodestats.dnode_hold_alloc_hits") { if s != zfsSysctl("kstat.zfs.misc.dnodestats.dnode_hold_alloc_hits") {
return return
@ -447,7 +448,7 @@ func TestDnodestatsParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(37617) { if v.(uint64) != uint64(37617) {
t.Fatalf("Incorrect value parsed from procfs dnodestats data") t.Fatalf("Incorrect value parsed from procfs dnodestats data")
} }
@ -475,7 +476,7 @@ func TestVdevMirrorstatsParsing(t *testing.T) {
} }
handlerCalled := false handlerCalled := false
err = c.parseProcfsFile(vdevMirrorStatsFile, "vdev_mirror_stats", func(s zfsSysctl, v uint64) { err = c.parseProcfsFile(vdevMirrorStatsFile, "vdev_mirror_stats", func(s zfsSysctl, v interface{}) {
if s != zfsSysctl("kstat.zfs.misc.vdev_mirror_stats.preferred_not_found") { if s != zfsSysctl("kstat.zfs.misc.vdev_mirror_stats.preferred_not_found") {
return return
@ -483,7 +484,7 @@ func TestVdevMirrorstatsParsing(t *testing.T) {
handlerCalled = true handlerCalled = true
if v != uint64(94) { if v.(uint64) != uint64(94) {
t.Fatalf("Incorrect value parsed from procfs vdev_mirror_stats data") t.Fatalf("Incorrect value parsed from procfs vdev_mirror_stats data")
} }

Loading…
Cancel
Save