netns icon indicating copy to clipboard operation
netns copied to clipboard

NewNamed should return error if the named ns exists

Open fortitudepub opened this issue 1 year ago • 2 comments

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.

fortitudepub avatar Dec 15 '23 10:12 fortitudepub

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.

jeffwidman avatar Dec 18 '23 09:12 jeffwidman

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.

vanytsvetkov avatar Aug 04 '24 21:08 vanytsvetkov