Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

agent: network: Routes and Interfaces list are unproperly modified when parsing them #226

Closed
sboeuf opened this issue Apr 26, 2018 · 0 comments · Fixed by #225
Closed

agent: network: Routes and Interfaces list are unproperly modified when parsing them #226

sboeuf opened this issue Apr 26, 2018 · 0 comments · Fixed by #225

Comments

@sboeuf
Copy link

sboeuf commented Apr 26, 2018

We need to make sure we don't modify a list that is being looped over by the caller. That's what happened with network.go when the list of routes and interfaces are removed by removeNetwork().

sboeuf pushed a commit to sboeuf/agent that referenced this issue Apr 26, 2018
When the pod is destroyed, it removes all the routes and interfaces
previously setup. The list of routes and interfaces is maintained
by the agent, referring to list of pointers. When the agent tries
to rely on this list to remove routes and interfaces, the list is
updated by the code removing them, leading to weird situations
where the loop does not properly go over the expected items of the
list.

This commit duplicates the list of routes and interfaces as non
pointers, so that we make sure that any removal from the function
UpdateRoute() or UpdateInterface() will not result in modifying
the initial list being parsed.

Fixes kata-containers#226

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/agent that referenced this issue Apr 26, 2018
When the pod is destroyed, it removes all the routes and interfaces
previously setup. The list of routes and interfaces is maintained
by the agent, referring to list of pointers. When the agent tries
to rely on this list to remove routes and interfaces, the list is
updated by the code removing them, leading to weird situations
where the loop does not properly go over the expected items of the
list.

This commit duplicates the list of routes and interfaces as non
pointers, so that we make sure that any removal from the function
UpdateRoute() or UpdateInterface() will not result in modifying
the initial list being parsed.

Fixes kata-containers#226

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/agent that referenced this issue Apr 27, 2018
By storing the network information such as routes and interfaces
as pointers, we end up having issues if we try to remove some
elements from the list as the same time this list is parsed through
a for loop.

This allows for a proper duplication of the data, so that we can
actually modify the global lists maintained by the pod, while we
loop over every item of the same lists.

Fixes kata-containers#226

Signed-off-by: Sebastien Boeuf <[email protected]>
sboeuf pushed a commit to sboeuf/agent that referenced this issue Apr 30, 2018
By storing the network information such as routes as pointers, we
end up having issues if we try to remove some elements from the list
as at the same time this list is parsed through by a loop.

For interfaces, we can use maps instead, because we have a way to
keep track of them based on their names. Maps are safer and allow
to keep pointers in this case.

In both cases, routes and interfaces list can be modified properly
while we loop over every items.

Fixes kata-containers#226

Signed-off-by: Sebastien Boeuf <[email protected]>
jshachm pushed a commit to jshachm/agent that referenced this issue Nov 22, 2018
By storing the network information such as routes as pointers, we
end up having issues if we try to remove some elements from the list
as at the same time this list is parsed through by a loop.

For interfaces, we can use maps instead, because we have a way to
keep track of them based on their names. Maps are safer and allow
to keep pointers in this case.

In both cases, routes and interfaces list can be modified properly
while we loop over every items.

Fixes kata-containers#226

Signed-off-by: Sebastien Boeuf <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant