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.

Sign up for GitHub

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

Jump to bottom

Cherry-pick #25073 to 7.12: Fix panic when Hearbeat monitor initialization fails twice #25090

Merged
merged 1 commit into from
Apr 14, 2021
Merged

Cherry-pick #25073 to 7.12: Fix panic when Hearbeat monitor initialization fails twice #25090

merged 1 commit into from
Apr 14, 2021

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Apr 14, 2021 •
edited by zube bot
Loading

Cherry-pick of PR #25073 to 7.12 branch. Original message:

What does this PR do?

Prevents a panic happening when ICMP initialization fails twice.

Initialization was only tried once. Second time a nil object and a nil error are returned, producing nil pointer dereferences later.

With static configurations the first failure inmediatelly stops heartbeat, so this is not a problem, but with autodiscover multiple initialization attempts may happen.

Why is it important?

Prevent unexpected panics when introducing ICMP configurations through autodiscover.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

How to test this PR locally

  • Configure two ICMP monitors using autodiscover without NET_RAW capabilities.
  • Failures should be printed, but Heartbeat should continue running.

Related issues

Logs

For the record, stack trace printed when this issue happens:

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

goroutine 74 [running]:
github.com/elastic/beats/v7/heartbeat/monitors/active/icmp.(*stdICMPLoop).checkNetworkMode(0x0, 0x29c9292, 0x2, 0x0, 0xc000100400)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/active/icmp/stdloop.go:170 +0x5d
github.com/elastic/beats/v7/heartbeat/monitors/active/icmp.(*jobFactory).makePlugin(0xc000954a20, 0x0, 0x0, 0x0, 0x0, 0x0, 0x2d0eda0, 0x3e69478)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/active/icmp/icmp.go:94 +0x7a
github.com/elastic/beats/v7/heartbeat/monitors/active/icmp.create(0x29ca5fc, 0x4, 0xc0008151a0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/active/icmp/icmp.go:62 +0x2b5
github.com/elastic/beats/v7/heartbeat/monitors/plugin.(*PluginFactory).Create(...)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/plugin/plugin.go:164
github.com/elastic/beats/v7/heartbeat/monitors.newMonitorUnsafe(0xc0008151a0, 0xc000010020, 0x0, 0x0, 0x0, 0x0, 0xab7702fa0e1a4b9e, 0x0, 0x0)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/monitor.go:160 +0x3af
github.com/elastic/beats/v7/heartbeat/monitors.newMonitor(0xc0008151a0, 0xc000010020, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc00061dc48, 0x14c2490)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/monitor.go:105 +0x6c
github.com/elastic/beats/v7/heartbeat/monitors.checkMonitorConfig(0xc0008151a0, 0xc000010020, 0x0, 0x0, 0xc000738140)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/monitor.go:76 +0x53
github.com/elastic/beats/v7/heartbeat/monitors.(*RunnerFactory).CheckConfig(0xc00011c000, 0xc0008151a0, 0x0, 0x0)
	/home/jaime/gocode/src/github.com/elastic/beats/heartbeat/monitors/factory.go:85 +0x47
github.com/elastic/beats/v7/libbeat/autodiscover.(*Autodiscover).handleStart(0xc00022a750, 0xc0008153b0, 0x29cbcf7)
	/home/jaime/gocode/src/github.com/elastic/beats/libbeat/autodiscover/autodiscover.go:210 +0x484
github.com/elastic/beats/v7/libbeat/autodiscover.(*Autodiscover).worker(0xc00022a750)
	/home/jaime/gocode/src/github.com/elastic/beats/libbeat/autodiscover/autodiscover.go:140 +0x4ea
created by github.com/elastic/beats/v7/libbeat/autodiscover.(*Autodiscover).Start
	/home/jaime/gocode/src/github.com/elastic/beats/libbeat/autodiscover/autodiscover.go:121 +0x111
jaime@voyager:~/gocode/src/github.com/elastic/beats/heartbeat$

@jsoriano
Fix panic when Hearbeat monitor initialization fails twice (elastic#2…
b053f18 
…5073)

Initialization was only tried once. Second time a nil object and a nil
error are returned, producing nil pointer dereferences later.

(cherry picked from commit 1d858bd)
@jsoriano jsoriano requested a review from a team as a code owner April 14, 2021 15:46
@jsoriano jsoriano added [zube]: In Review backport Team:Integrations Label for the Integrations team Team:obs-ds-hosted-services Label for the Observability Hosted Services team labels Apr 14, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/uptime (Team:Uptime)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Apr 14, 2021
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #25090 opened

  • Start Time: 2021-04-14T15:47:02.287+0000

  • Duration: 53 min 56 sec

  • Commit: b053f18

Test stats 🧪

Test Results
Failed 0
Passed 3303
Skipped 76
Total 3379

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 3303
Skipped 76
Total 3379

@jsoriano jsoriano merged commit 7b1d41b into elastic:7.12 Apr 14, 2021
@jsoriano jsoriano deleted the backport_25073_7.12 branch April 14, 2021 16:57
@zube zube bot removed the [zube]: Done label Jul 14, 2021
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@jsoriano
Fix panic when Hearbeat monitor initialization fails twice (elastic#2…
a43a26f 
…5073) (elastic#25090)

Initialization was only tried once. Second time a nil object and a nil
error are returned, producing nil pointer dereferences later.

(cherry picked from commit 841c82d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
backport Team:Integrations Label for the Integrations team Team:obs-ds-hosted-services Label for the Observability Hosted Services team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jsoriano @elasticmachine