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

hubble/peer: fix buf.Pop() crash issue #12257

Merged
merged 1 commit into from Jun 24, 2020

Conversation

Jianlin-lv
Copy link
Contributor

While waiting for b.mu.Lock, b.buffer be closed that is
triggered when the maximum buffer size is reached,
So check b.stop channel before access b.buf.

Signed-off-by: Jianlin Lv Jianlin.Lv@arm.com

Fixes: #12178

While waiting for b.mu.Lock, b.buffer be closed that is
triggered when the maximum buffer size is reached,
So check b.stop channel before access b.buf.

Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
@Jianlin-lv Jianlin-lv requested a review from a team June 24, 2020 10:33
@maintainer-s-little-helper
Copy link

Please set the appropriate release note label.

@rolinh
Copy link
Member

rolinh commented Jun 24, 2020

LGTM, thanks for the fix!

@rolinh rolinh added sig/hubble Impacts hubble server or relay release-note/bug This PR fixes an issue in a previous release of Cilium. labels Jun 24, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.8.0 Jun 24, 2020
@gandro
Copy link
Member

gandro commented Jun 24, 2020

test-me-please

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 37.162% when pulling e41ab0a on Jianlin-lv:pr-fix-peer-crash into 55dd1b9 on cilium:master.

@rolinh
Copy link
Member

rolinh commented Jun 24, 2020

@Jianlin-lv Would you mind adding a unit test to cover this case? It should be pretty easy.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 24, 2020
@Jianlin-lv
Copy link
Contributor Author

@Jianlin-lv Would you mind adding a unit test to cover this case? It should be pretty easy.
PR #12269 adds a test case for this scenario.

@gandro gandro merged commit c75df25 into cilium:master Jun 24, 2020
1.8.0 automation moved this from In progress to Merged Jun 24, 2020
This was referenced Jun 26, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.8 in 1.8.0 Jun 26, 2020
@joestringer joestringer removed this from Merged in 1.8.0 Jun 26, 2020
@joestringer joestringer removed this from Backport pending to v1.8 in 1.8.0 Jun 26, 2020
@joestringer joestringer added this to Backport pending to v1.8 in 1.8.1 Jun 26, 2020
@joestringer joestringer moved this from Backport pending to v1.8 to Backport done to v1.8 in 1.8.1 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/hubble Impacts hubble server or relay
Projects
No open projects
1.8.1
Backport done to v1.8
7 participants