Skip to content

fix(datarace): Fix possible nil pointer dereference#11804

Merged
aanm merged 1 commit intocilium:masterfrom
sayboras:bugfix/nil-check
Jun 2, 2020
Merged

fix(datarace): Fix possible nil pointer dereference#11804
aanm merged 1 commit intocilium:masterfrom
sayboras:bugfix/nil-check

Conversation

@sayboras
Copy link
Copy Markdown
Member

@sayboras sayboras commented Jun 1, 2020

This commit is to fix the recent changes after rebased. Related PR
is #11685.

Basically, there was a check if Node is nil after calling mutex lock,
which will cause nil pointer dereference. I just refactor the code
to make sure underlying node is not nil before obtaining mutex lock.

Relates to 5259ff7

Signed-off-by: Tam Mach sayboras@yahoo.com

fix(datarace): Fix possible nil pointer dereference

@sayboras sayboras requested a review from a team as a code owner June 1, 2020 23:07
@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

This commit is to fix the recent changes after rebased. Related PR
is cilium#11685.

Basically, there was a check if Node is nil after calling mutex lock,
which will cause nil pointer dereference. I just refactor the code
to make sure underlying node is not nil before obtaining mutex lock.

Relates to 5259ff7

Signed-off-by: Tam Mach <sayboras@yahoo.com>
@sayboras sayboras force-pushed the bugfix/nil-check branch from a552790 to 5df72a9 Compare June 1, 2020 23:09
@christarazi christarazi added area/eni Impacts ENI based IPAM. kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.7 and removed dont-merge/needs-release-note labels Jun 1, 2020
@christarazi
Copy link
Copy Markdown
Member

christarazi commented Jun 1, 2020

test-me-please

Edit: net-next provisioning failure

Copy link
Copy Markdown
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

🎉

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 1, 2020

Coverage Status

Coverage increased (+0.03%) to 36.93% when pulling 5df72a9 on sayboras:bugfix/nil-check into b7be1c0 on cilium:master.

@christarazi
Copy link
Copy Markdown
Member

retest-net-next

@sayboras
Copy link
Copy Markdown
Member Author

sayboras commented Jun 2, 2020

@christarazi @pchaigno seems like the required jobs are passed now, please let me know if anything is required. Thanks.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 2, 2020
@aanm aanm merged commit e8a7de6 into cilium:master Jun 2, 2020
@sayboras sayboras deleted the bugfix/nil-check branch June 2, 2020 09:23
@christarazi
Copy link
Copy Markdown
Member

Removing the need to backport this to 1.7 as this race doesn't exist in the 1.7 tree. I erroneously thought that it would affect the 1.7 tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/eni Impacts ENI based IPAM. kind/cleanup This includes no functional changes. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants