Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memory corruption when number of filesystems > 16 #900

Conversation

juergenhoetzel
Copy link
Contributor

Fixes this crash:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x82879d]

goroutine 10 [running]:
runtime: unexpected return pc for github.com/prometheus/node_exporter/collector.(*filesystemCollector).GetStats called from 0x0
stack: frame={sp:0xc42019d810, fp:0xc42019fac8} stack=[0xc42019c000,0xc4201a0000)
000000c42019d710:  00000008016284d0  0100000000453630 
000000c42019d720:  0000000000000000  0000000000000001 
.....
000000c42019dff0:  0000000000000000  0000000000000000 
000000c42019e000:  0000000000000000  0000000000000000 
github.com/prometheus/node_exporter/collector.(*filesystemCollector).GetStats(0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
        /usr/home/juergen/go/src/github.com/prometheus/node_exporter/collector/filesystem_freebsd.go:59 +0x21d
created by github.com/prometheus/node_exporter/collector.nodeCollector.Collect
        /usr/home/juergen/go/src/github.com/prometheus/node_exporter/collector/collector.go:121 +0x109

The doubling of the array size (in the for loop) doesn't help. The getfsstat has already written beyond the end of the array. The correct way is to call getfsstat with a nil/null parameter to get the number of entries.

@juergenhoetzel juergenhoetzel force-pushed the fix-getfsstat-memory-corruption branch from 2282d8d to 483111b Compare April 14, 2018 19:26
@SuperQ
Copy link
Member

SuperQ commented Apr 15, 2018

Good catch. I just merged #898, which should fix the build errors for you.

@juergenhoetzel juergenhoetzel force-pushed the fix-getfsstat-memory-corruption branch from 483111b to 222327b Compare April 15, 2018 16:34
@juergenhoetzel
Copy link
Contributor Author

Good catch. I just merged #898, which should fix the build errors for you.

Thanks. I rebased. I'm not sure what failed on buidkite.

return nil, err
}
buf := make([]unix.Statfs_t, n)
n, err = unix.Getfsstat(buf, noWait)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build failure says this should be _, err = ....

collector/filesystem_freebsd.go:49:2: this value of n is never used (SA4006)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, no, that's wrong. It should be buf, err = ....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, no, that's wrong. It should be buf, err = ...

RETURN VALUES
     Upon successful completion, the number of statfs structures is returned.
     Otherwise, -1 is returned and the global variable errno is set to
     indicate the error.

@juergenhoetzel juergenhoetzel force-pushed the fix-getfsstat-memory-corruption branch from 222327b to 4a63cb2 Compare April 15, 2018 17:03
@juergenhoetzel
Copy link
Contributor Author

Thanks. Fixed it.

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@discordianfish discordianfish merged commit de0632c into prometheus:master Apr 16, 2018
@discordianfish
Copy link
Member

LGTM

oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants