Jelajahi Sumber

linux: let the kernel resolve the tun.dev %d template atomically

Resolving the template in userspace (LinkList then TUNSETIFF) races with
concurrent instances: both can pick nebula0, and with multiqueue the loser
silently attaches as a second queue to the winner's device instead of
failing. TUNSETIFF natively substitutes a single %d via dev_alloc_name,
atomically and always allocating a fresh device, so keep the fail-fast
validation but pass the template through and read the resolved name back.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fg7oD3uZ4jZjjQ4gRsPAwB
JackDoan 3 bulan lalu
induk
melakukan
bc04261d0b
3 mengubah file dengan 17 tambahan dan 106 penghapusan
  1. 1 1
      examples/config.yml
  2. 12 49
      overlay/tun_linux.go
  3. 4 56
      overlay/tun_linux_test.go

+ 1 - 1
examples/config.yml

@@ -245,7 +245,7 @@ tun:
   # For Linux: a single `%d` anywhere in the name is treated as a template and replaced with the
   #   lowest number that yields an unused device name (e.g. `nebula%d` becomes `nebula0`, then `nebula1`, and so on, `neb%dprod` becomes `neb0prod`).
   #   Only on Linux: `nebula%d` is the default if tun.dev is unset.
-  #   The resulting name must be shorter than the kernel limit of 16 characters.
+  #   The name, both before and after %d substitution, must be shorter than the kernel limit of 16 characters.
   # For macOS: if set, must be in the form `utun[0-9]+`.
   # For NetBSD: Required to be set, must be in the form `tun[0-9]+`
   dev: nebula1

+ 12 - 49
overlay/tun_linux.go

@@ -251,10 +251,14 @@ func newTunFromFd(c *config.C, l *slog.Logger, deviceFd int, vpnNetworks []netip
 }
 
 func newTun(c *config.C, l *slog.Logger, vpnNetworks []netip.Prefix, multiqueue bool) (*tun, error) {
-	// Resolve (and validate) the device name up front so a bad tun.dev fails
-	// fast, before we open /dev/net/tun or leak a file descriptor.
-	tunName, err := findNextTunName(c.GetString("tun.dev", "nebula%d"))
-	if err != nil {
+	// Validate the device name up front so a bad tun.dev fails fast, before we
+	// open /dev/net/tun or leak a file descriptor. A single %d in the name is
+	// substituted by the kernel during TUNSETIFF (dev_alloc_name) with the
+	// lowest number that yields an unused device name. Resolving the template
+	// in the kernel keeps the pick-a-name/create-the-device pair atomic, so
+	// concurrent callers can never race each other to the same name.
+	tunName := c.GetString("tun.dev", "nebula%d")
+	if err := validateTunName(tunName); err != nil {
 		return nil, err
 	}
 
@@ -318,55 +322,14 @@ func validateTunName(tunName string) error {
 	if tunName == "%d" {
 		return errors.New("please don't name your tun device '%d'")
 	}
-	// The shortest name a template can produce replaces %d with a single digit;
-	// if even that is not shorter than IFNAMSIZ the template can never yield a
-	// usable name.
-	if len(tunName)-len("%d")+len("0") >= unix.IFNAMSIZ {
-		return fmt.Errorf("tun.dev template %q would result in a name that is not shorter than the maximum device name length of %d", tunName, unix.IFNAMSIZ)
+	// The kernel substitutes the %d itself and requires the template, like a
+	// literal name, to be NUL-terminated within IFNAMSIZ bytes.
+	if len(tunName) >= unix.IFNAMSIZ {
+		return fmt.Errorf("tun.dev template %q is not shorter than the maximum device name length of %d", tunName, unix.IFNAMSIZ)
 	}
 	return nil
 }
 
-// findNextTunName resolves a tun.dev value into a concrete device name. A value
-// without a "%d" is returned unchanged; a "%d" placeholder (anywhere in the
-// name) has the lowest unused integer substituted in based on the devices
-// currently present.
-func findNextTunName(tunName string) (string, error) {
-	if err := validateTunName(tunName); err != nil {
-		return "", err
-	}
-	if !strings.Contains(tunName, "%d") {
-		return tunName, nil
-	}
-
-	links, err := netlink.LinkList()
-	if err != nil {
-		return "", err
-	}
-	used := make(map[string]struct{}, len(links))
-	for _, link := range links {
-		used[link.Attrs().Name] = struct{}{}
-	}
-	return nextTunName(tunName, used)
-}
-
-// nextTunName substitutes the lowest unused integer into a template's "%d"
-// placeholder, skipping any name present in used. tunName is assumed to have
-// already passed validateTunName (exactly one "%d", room for a digit). It errors
-// only if every candidate that is shorter than IFNAMSIZ is already taken.
-func nextTunName(tunName string, used map[string]struct{}) (string, error) {
-	prefix, suffix, _ := strings.Cut(tunName, "%d")
-	for i := 0; ; i++ {
-		candidateName := fmt.Sprintf("%s%d%s", prefix, i, suffix)
-		if len(candidateName) >= unix.IFNAMSIZ {
-			return "", fmt.Errorf("all device names matching template %q shorter than the maximum length of %d are already in use", tunName, unix.IFNAMSIZ)
-		}
-		if _, taken := used[candidateName]; !taken {
-			return candidateName, nil
-		}
-	}
-}
-
 // newTunGeneric does all the stuff common to different tun initialization paths. It will close your files on error.
 func newTunGeneric(c *config.C, l *slog.Logger, fd int, vpnNetworks []netip.Prefix) (*tun, error) {
 	tfd, err := newTunFd(fd)

+ 4 - 56
overlay/tun_linux_test.go

@@ -38,14 +38,6 @@ func TestTunAdvMSS(t *testing.T) {
 	}
 }
 
-func nameSet(names ...string) map[string]struct{} {
-	used := make(map[string]struct{}, len(names))
-	for _, n := range names {
-		used[n] = struct{}{}
-	}
-	return used
-}
-
 func TestValidateTunName(t *testing.T) {
 	// A device name must be shorter than IFNAMSIZ (i.e. IFNAMSIZ-1 chars max).
 	maxLenName := strings.Repeat("a", unix.IFNAMSIZ-1)
@@ -61,11 +53,12 @@ func TestValidateTunName(t *testing.T) {
 		{"trailing template is fine", "nebula%d", false},
 		{"mid-string template is fine", "neb%dprod", false},
 		{"leading template is fine", "%dnebula", false},
-		{"template at the max static length is fine", strings.Repeat("a", unix.IFNAMSIZ-2) + "%d", false},
+		{"template at the max length is fine", strings.Repeat("a", unix.IFNAMSIZ-3) + "%d", false},
+		{"template at IFNAMSIZ is rejected", strings.Repeat("a", unix.IFNAMSIZ-2) + "%d", true},
 		{"bare %d is rejected", "%d", true},
 		{"multiple %d is rejected", "neb%d%dprod", true},
-		{"template with no room for a digit is rejected", strings.Repeat("a", unix.IFNAMSIZ-1) + "%d", true},
-		{"mid-string template with no room for a digit is rejected", "neb%d" + strings.Repeat("a", unix.IFNAMSIZ-3), true},
+		{"over-long template is rejected", strings.Repeat("a", unix.IFNAMSIZ-1) + "%d", true},
+		{"over-long mid-string template is rejected", "neb%d" + strings.Repeat("a", unix.IFNAMSIZ-3), true},
 	}
 
 	for _, tt := range tests {
@@ -80,48 +73,3 @@ func TestValidateTunName(t *testing.T) {
 		})
 	}
 }
-
-func TestNextTunName(t *testing.T) {
-	// A prefix long enough that only single-digit suffixes (0-9) fit within
-	// IFNAMSIZ, so marking all ten used exercises running out of names.
-	longPrefix := strings.Repeat("a", unix.IFNAMSIZ-2)
-	longUsed := make([]string, 0, 10)
-	for i := 0; i < 10; i++ {
-		longUsed = append(longUsed, longPrefix+string(rune('0'+i)))
-	}
-
-	tests := []struct {
-		name    string
-		tmpl    string
-		used    map[string]struct{}
-		want    string
-		wantErr bool
-	}{
-		{"nothing used picks zero", "nebula%d", nil, "nebula0", false},
-		{"skips taken names", "nebula%d", nameSet("nebula0", "nebula1"), "nebula2", false},
-		{"picks the lowest free index", "nebula%d", nameSet("nebula0", "nebula2"), "nebula1", false},
-		{"ignores unrelated names", "nebula%d", nameSet("eth0", "tun5"), "nebula0", false},
-		{"mid-string placeholder picks zero", "neb%dprod", nil, "neb0prod", false},
-		{"mid-string placeholder skips taken", "neb%dprod", nameSet("neb0prod", "neb1prod"), "neb2prod", false},
-		{"leading placeholder picks zero", "%dnebula", nameSet("tun0"), "0nebula", false},
-		{"runs out of names within IFNAMSIZ", longPrefix + "%d", nameSet(longUsed...), "", true},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := nextTunName(tt.tmpl, tt.used)
-			if tt.wantErr {
-				if err == nil {
-					t.Fatalf("expected an error, got name %q", got)
-				}
-				return
-			}
-			if err != nil {
-				t.Fatalf("unexpected error: %v", err)
-			}
-			if got != tt.want {
-				t.Errorf("got %q, want %q", got, tt.want)
-			}
-		})
-	}
-}