Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

networkdb: fix race in deleteNetwork #1503

Merged
merged 1 commit into from
Oct 13, 2016
Merged

networkdb: fix race in deleteNetwork #1503

merged 1 commit into from
Oct 13, 2016

Conversation

LK4D4
Copy link
Contributor

@LK4D4 LK4D4 commented Oct 11, 2016

There are multiple places which read from that slice(i.e. bulkSync).

@aboch
Copy link
Contributor

aboch commented Oct 12, 2016

CI failure is legitimate:

# github.com/docker/libnetwork/networkdb
networkdb/networkdb.go:501: invalid operation: name == nodeName (mismatched types int and string)
networkdb/networkdb.go:504: cannot use name (type int) as type string in append

There are multiple places which reads from that slice(i.e. bulkSync).

Signed-off-by: Alexander Morozov <lk4d4math@gmail.com>
@LK4D4
Copy link
Contributor Author

LK4D4 commented Oct 12, 2016

@aboch fixed! Sorry for that.

@aboch
Copy link
Contributor

aboch commented Oct 12, 2016

I see that func (nDB *NetworkDB) deleteNetworkNode() is always called under nDB lock. The same is true for the counterpart function func (nDB *NetworkDB) addNetworkNode().
Which makes me think this is why no precaution is taken inside these functions while manipulating the map content.

How would a race be possible, can you point me to the code path. Thanks.

@LK4D4
Copy link
Contributor Author

LK4D4 commented Oct 12, 2016

@aboch Sure, nodes slice is not protected by anything, so it can be modified in deleteNetworkNode and read in https://github.com/docker/libnetwork/blob/master/networkdb/cluster.go#L416 for example. And it's not a single usage of that slice as I see.
I think that addNetworkNode is probably racy too, but it's not so obvious.

@aboch
Copy link
Contributor

aboch commented Oct 12, 2016

Oh I see, the code allows to get the nodes slice and manipulate it outside of these two functions.
Thanks for the pointer.

LGTM

@sanimej
Copy link

sanimej commented Oct 13, 2016

LGTM

@sanimej sanimej merged commit ebfa69e into moby:master Oct 13, 2016
@LK4D4 LK4D4 deleted the fix_race_delete_network branch October 13, 2016 20:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants