Don't re-initialize package level var native to fix data race #637
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@aboch PTAL |
tklauser
force-pushed
the
fix-data-race-native
branch
3 times, most recently
from
b8cc0c9
to
f7d465c
Compare
|
👍 |
|
pls push again so CI is triggered |
|
LGTM |
tklauser
force-pushed
the
fix-data-race-native
branch
from
f7d465c
to
0d1deea
Compare
done, rebased and pushed again |
|
please squash your two commits into one |
tklauser
force-pushed
the
fix-data-race-native
branch
from
0d1deea
to
438a16d
Compare
done |
The package level var native (holding the native endianness) is initialized at package load time. Thus there is no need to re-initalize it in functions using it, e.g. (*Handle).filterModify, parseU32Data, parseFwData, parseBpfData and parseMatchAllData. This fixes a data race between these functions and any read access of var native, e.g. in LinkDeserialize as reported in issue vishvananda#633. Also don't re-declare local variables shadowing the global package-level var. Fixes vishvananda#633 Signed-off-by: Tobias Klauser <tobias@cilium.io>
tklauser
force-pushed
the
fix-data-race-native
branch
from
438a16d
to
8bc9d0c
Compare
tklauser
added a commit
to tklauser/netlink
that referenced
this pull request
May 10, 2021
…hOptions This was missed in vishvananda#637 due to it being introduced by vishvananda#623, which was merged just recently. Signed-off-by: Tobias Klauser <tobias@cilium.io>
tklauser
added a commit
to cilium/cilium
that referenced
this pull request
May 10, 2021
This pulls in vishvananda/netlink#637 and vishvananda/netlink#648 with data race fixes. Fixes #15438 Signed-off-by: Tobias Klauser <tobias@cilium.io>
jrajahalme
pushed a commit
to cilium/cilium
that referenced
this pull request
May 11, 2021
This pulls in vishvananda/netlink#637 and vishvananda/netlink#648 with data race fixes. Fixes #15438 Signed-off-by: Tobias Klauser <tobias@cilium.io>
glibsm
pushed a commit
to glibsm/cilium
that referenced
this pull request
May 12, 2021
[ upstream commit f7bd191 ] This pulls in vishvananda/netlink#637 and vishvananda/netlink#648 with data race fixes. Fixes cilium#15438 Signed-off-by: Tobias Klauser <tobias@cilium.io> Signed-off-by: Glib Smaga <code@gsmaga.com>
aanm
pushed a commit
to cilium/cilium
that referenced
this pull request
May 13, 2021
[ upstream commit f7bd191 ] This pulls in vishvananda/netlink#637 and vishvananda/netlink#648 with data race fixes. Fixes #15438 Signed-off-by: Tobias Klauser <tobias@cilium.io> Signed-off-by: Glib Smaga <code@gsmaga.com>
gkodali-zededa
pushed a commit
to gkodali-zededa/netlink
that referenced
this pull request
May 21, 2021
…hOptions This was missed in vishvananda#637 due to it being introduced by vishvananda#623, which was merged just recently. Signed-off-by: Tobias Klauser <tobias@cilium.io>
cyberys
pushed a commit
to cyberys/netlink
that referenced
this pull request
Apr 5, 2022
…hOptions This was missed in vishvananda#637 due to it being introduced by vishvananda#623, which was merged just recently. Signed-off-by: Tobias Klauser <tobias@cilium.io>
cjmakes
pushed a commit
to cjmakes/netlink
that referenced
this pull request
Jun 29, 2022
…hOptions This was missed in vishvananda#637 due to it being introduced by vishvananda#623, which was merged just recently. Signed-off-by: Tobias Klauser <tobias@cilium.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The package level
var native(holding the native endianness) isinitialized at package load time. Thus there is no need to re-initalize
it in
(*Handle).filterModify,parseU32Data,parseFwData,parseBpfDataand
parseMatchAllData. This fixes a data race between these functionsand any read access of
var native, e.g. inLinkDeserializeas reportedin issue #633.
The second commit, while not directly related to the data race drops local re-declarations of
var nativeusingnative := NativeEndian(). Instead the package-level global var can be usedFixes #633