Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

NewNamed should return error if the named ns exists #79

Open
fortitudepub opened this issue Dec 15, 2023 · 2 comments
Open

NewNamed should return error if the named ns exists #79

fortitudepub opened this issue Dec 15, 2023 · 2 comments

Comments

@fortitudepub
Copy link

Dear maintainer,

If the names ns exits, the NewNamed function will still call New() which will call unshare syscall, but in the following os.OpenFile it will return error due to the ns file exists.

This will result the calling thread's namespace changed to an unknown netns, cause hard-to-debug issue.

It seems better to return error eary to prevent such issue.

@jeffwidman
Copy link
Collaborator

I haven't looked at the underlying syscalls in a while, is there a way to atomically identify that the namespace exists at time of creation? Ie, avoid the data race between "check if namespace already exists" and "create new namespace"?

If not, then I'd probably prefer to clearly document that we don't check and recommend the user first check themselves. That way it's more clear in calling code that the potential for a race exists there rather than burying it within this library. Not a strongly held opinion, just what seems more intuitive at first glance.

vanytsvetkov added a commit to vanytsvetkov/netns that referenced this issue Aug 4, 2024
@vanytsvetkov
Copy link

Hm... I agree with Mr. fortitudepub.
I checked the source code of iproute2/ip (see function netns_add), and made sure that the request ip netns add 0xbadbeef really first "checks" the existence of a possible duplicate netns along the path /var/run/netns/0xbadbeef, and only then performs an unshare syscall to create a new namespace.
I suggest not to deviate from the logic of the classics, and correct the flaw pointed out by the author of the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants