Skip to content

azure/ipam: Fix nil dereference with logger#11786

Merged
christarazi merged 1 commit intocilium:masterfrom
christarazi:pr/christarazi/fix-nil-ptr-azure-ipam
May 30, 2020
Merged

azure/ipam: Fix nil dereference with logger#11786
christarazi merged 1 commit intocilium:masterfrom
christarazi:pr/christarazi/fix-nil-ptr-azure-ipam

Conversation

@christarazi
Copy link
Copy Markdown
Member

The code erroneously used log which is a global variable, that's
unprotected by a mutex. The correct usage is to use scopedLog which is
passed into ResyncInterfacesAndIPs(), which already has been locked by
the caller of ResyncInterfacesAndIPs(). The caller is n.recalculate().

Fixes: #11785
Fixes: 3dfd638 ("ipam: Move iterator logic into generic InstanceMap")
Fixes: 9779b09 ("ipam: Support for Azure IPAM")

The code erroneously used `log` which is a global variable, that's
unprotected by a mutex. The correct usage is to use `scopedLog` which is
passed into ResyncInterfacesAndIPs(), which already has been locked by
the caller of ResyncInterfacesAndIPs(). The caller is n.recalculate().

Fixes: cilium#11785
Fixes: 3dfd638 ("ipam: Move iterator logic into generic InstanceMap")
Fixes: 9779b09 ("ipam: Support for Azure IPAM")

Signed-off-by: Chris Tarazi <chris@isovalent.com>
@christarazi christarazi requested a review from a team as a code owner May 29, 2020 21:37
@christarazi christarazi added area/ipam kind/bug This is a bug in the Cilium logic. release-note/misc This PR makes changes that have no direct user impact. area/azure Impacts Azure based IPAM. labels May 29, 2020
@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@christarazi
Copy link
Copy Markdown
Member Author

test-me-please

Copy link
Copy Markdown
Member

@ungureanuvladvictor ungureanuvladvictor left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

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

let's roll

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.03%) to 36.813% when pulling 2e1ddd5 on christarazi:pr/christarazi/fix-nil-ptr-azure-ipam into 138d556 on cilium:master.

@christarazi
Copy link
Copy Markdown
Member Author

Travis hit #11560

@christarazi christarazi merged commit b5ed91d into cilium:master May 30, 2020
@christarazi christarazi deleted the pr/christarazi/fix-nil-ptr-azure-ipam branch May 30, 2020 05:41
@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 May 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/azure Impacts Azure based IPAM. kind/bug This is a bug in the Cilium logic. 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.

ipam: azure: Nil pointer dereference in IPAMSuite.TestIpamManyNodes

5 participants