cmd/link: disallow pull-only linknames

As mentioned in CL 584598, linkname is a mechanism that, when
abused, can break API integrity and even safety of Go programs.
CL 584598 is a first step to restrict the use of linknames, by
implementing a blocklist. This CL takes a step further, tightening
up the restriction by allowing linkname references ("pull") only
when the definition side explicitly opts into it, by having a
linkname on the definition (possibly to itself). This way, it is at
least clear on the definition side that the symbol, despite being
unexported, is accessed outside of the package. Unexported symbols
without linkname can now be actually private. This is similar to
the symbol visibility rule used by gccgo for years (which defines
unexported non-linknamed symbols as C static symbols).

As there can be pull-only linknames in the wild that may be broken
by this change, we currently only enforce this rule for symbols
defined in the standard library. Push linknames are added in the
standard library to allow things build.

Linkname references to external (non-Go) symbols are still allowed,
as their visibility is controlled by the C symbol visibility rules
and enforced by the C (static or dynamic) linker.

Assembly symbols are treated similar to linknamed symbols.

This is controlled by -checklinkname linker flag, currently not
enabled by default. A follow-up CL will enable it by default.

Change-Id: I07344f5c7a02124dbbef0fbc8fec3b666a4b2b0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/585358
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>

14 files changed, 127 insertions, 56 deletions

diff --git a/src/cmd/compile/internal/base/flag.go b/src/cmd/compile/internal/base/flag.go
index f514ce104a..fe515aafbf 100644
--- a/src/cmd/compile/internal/base/flag.go
+++ b/src/cmd/compile/internal/base/flag.go
@@ -213,6 +213,8 @@ func ParseFlags() {
 		Flag.CompilingRuntime = true
 	}
 
+	Ctxt.Std = Flag.Std
+
 	// Three inputs govern loop iteration variable rewriting, hash, experiment, flag.
 	// The loop variable rewriting is:
 	// IF non-empty hash, then hash determines behavior (function+line match) (*)
diff --git a/src/cmd/compile/internal/liveness/plive.go b/src/cmd/compile/internal/liveness/plive.go
index dd48d10bc5..1a36035f46 100644
--- a/src/cmd/compile/internal/liveness/plive.go
+++ b/src/cmd/compile/internal/liveness/plive.go
@@ -1551,6 +1551,7 @@ func WriteFuncMap(fn *ir.Func, abiInfo *abi.ABIParamResultInfo) {
 		nbitmap = 2
 	}
 	lsym := base.Ctxt.Lookup(fn.LSym.Name + ".args_stackmap")
+	lsym.Set(obj.AttrLinkname, true) // allow args_stackmap referenced from assembly
 	off := objw.Uint32(lsym, 0, uint32(nbitmap))
 	off = objw.Uint32(lsym, off, uint32(bv.N))
 	off = objw.BitVec(lsym, off, bv)
diff --git a/src/cmd/compile/internal/ssagen/abi.go b/src/cmd/compile/internal/ssagen/abi.go
index 5c4a8aff69..d5ae3b1793 100644
--- a/src/cmd/compile/internal/ssagen/abi.go
+++ b/src/cmd/compile/internal/ssagen/abi.go
@@ -148,6 +148,11 @@ func (s *SymABIs) GenABIWrappers() {
 			// offsets to dispatch arguments, which currently using ABI0
 			// frame layout. Pin it to ABI0.
 			fn.ABI = obj.ABI0
+			// Propagate linkname attribute, which was set on the ABIInternal
+			// symbol.
+			if sym.Linksym().IsLinkname() {
+				sym.LinksymABI(fn.ABI).Set(obj.AttrLinkname, true)
+			}
 		}
 
 		// If cgo-exported, add the definition ABI to the cgo
diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index 5e5d539033..a5ce22c0c3 100644
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -1058,7 +1058,7 @@ func TestGoListDeps(t *testing.T) {
 	if runtime.Compiler != "gccgo" {
 		// Check the list is in dependency order.
 		tg.run("list", "-deps", "math")
-		want := "internal/cpu\nunsafe\nmath/bits\nmath\n"
+		want := "unsafe\ninternal/cpu\nmath/bits\nmath\n"
 		out := tg.stdout.String()
 		if !strings.Contains(out, "internal/cpu") {
 			// Some systems don't use internal/cpu.
diff --git a/src/cmd/internal/goobj/objfile.go b/src/cmd/internal/goobj/objfile.go
index fb87b04412..56ce76ad09 100644
--- a/src/cmd/internal/goobj/objfile.go
+++ b/src/cmd/internal/goobj/objfile.go
@@ -284,6 +284,7 @@ const (
 	_                               // was ObjFlagNeedNameExpansion
 	ObjFlagFromAssembly             // object is from asm src, not go
 	ObjFlagUnlinkable               // unlinkable package (linker will emit an error)
+	ObjFlagStd                      // standard library package
 )
 
 // Sym.Flag
@@ -304,6 +305,7 @@ const (
 	SymFlagDict
 	SymFlagPkgInit
 	SymFlagLinkname
+	SymFlagABIWrapper
 )
 
 // Returns the length of the name of the symbol.
@@ -336,6 +338,7 @@ func (s *Sym) IsItab() bool        { return s.Flag2()&SymFlagItab != 0 }
 func (s *Sym) IsDict() bool        { return s.Flag2()&SymFlagDict != 0 }
 func (s *Sym) IsPkgInit() bool     { return s.Flag2()&SymFlagPkgInit != 0 }
 func (s *Sym) IsLinkname() bool    { return s.Flag2()&SymFlagLinkname != 0 }
+func (s *Sym) ABIWrapper() bool    { return s.Flag2()&SymFlagABIWrapper != 0 }
 
 func (s *Sym) SetName(x string, w *Writer) {
 	binary.LittleEndian.PutUint32(s[:], uint32(len(x)))
@@ -882,3 +885,4 @@ func (r *Reader) Flags() uint32 {
 func (r *Reader) Shared() bool       { return r.Flags()&ObjFlagShared != 0 }
 func (r *Reader) FromAssembly() bool { return r.Flags()&ObjFlagFromAssembly != 0 }
 func (r *Reader) Unlinkable() bool   { return r.Flags()&ObjFlagUnlinkable != 0 }
+func (r *Reader) Std() bool          { return r.Flags()&ObjFlagStd != 0 }
diff --git a/src/cmd/internal/obj/link.go b/src/cmd/internal/obj/link.go
index 38869f0f47..647a459d59 100644
--- a/src/cmd/internal/obj/link.go
+++ b/src/cmd/internal/obj/link.go
@@ -1048,6 +1048,7 @@ type Link struct {
 	InParallel    bool // parallel backend phase in effect
 	UseBASEntries bool // use Base Address Selection Entries in location lists and PC ranges
 	IsAsm         bool // is the source assembly language, which may contain surprising idioms (e.g., call tables)
+	Std           bool // is standard library package
 
 	// state for writing objects
 	Text []*LSym
diff --git a/src/cmd/internal/obj/objfile.go b/src/cmd/internal/obj/objfile.go
index 648aae4fa2..2ed98cb577 100644
--- a/src/cmd/internal/obj/objfile.go
+++ b/src/cmd/internal/obj/objfile.go
@@ -57,6 +57,9 @@ func WriteObjFile(ctxt *Link, b *bio.Writer) {
 	if ctxt.IsAsm {
 		flags |= goobj.ObjFlagFromAssembly
 	}
+	if ctxt.Std {
+		flags |= goobj.ObjFlagStd
+	}
 	h := goobj.Header{
 		Magic:       goobj.Magic,
 		Fingerprint: ctxt.Fingerprint,
@@ -309,6 +312,7 @@ func (w *writer) StringTable() {
 const cutoff = int64(2e9) // 2 GB (or so; looks better in errors than 2^31)
 
 func (w *writer) Sym(s *LSym) {
+	name := s.Name
 	abi := uint16(s.ABI())
 	if s.Static() {
 		abi = goobj.SymABIstatic
@@ -348,10 +352,15 @@ func (w *writer) Sym(s *LSym) {
 	if s.IsPkgInit() {
 		flag2 |= goobj.SymFlagPkgInit
 	}
-	if s.IsLinkname() || w.ctxt.IsAsm { // assembly reference is treated the same as linkname
+	if s.IsLinkname() || (w.ctxt.IsAsm && name != "") || name == "main.main" {
+		// Assembly reference is treated the same as linkname,
+		// but not for unnamed (aux) symbols.
+		// The runtime linknames main.main.
 		flag2 |= goobj.SymFlagLinkname
 	}
-	name := s.Name
+	if s.ABIWrapper() {
+		flag2 |= goobj.SymFlagABIWrapper
+	}
 	if strings.HasPrefix(name, "gofile..") {
 		name = filepath.ToSlash(name)
 	}
diff --git a/src/cmd/link/internal/ld/lib.go b/src/cmd/link/internal/ld/lib.go
index cb0961eaef..11df3a466d 100644
--- a/src/cmd/link/internal/ld/lib.go
+++ b/src/cmd/link/internal/ld/lib.go
@@ -518,6 +518,9 @@ func (ctxt *Link) findLibPath(libname string) string {
 
 func (ctxt *Link) loadlib() {
 	var flags uint32
+	if *flagCheckLinkname {
+		flags |= loader.FlagCheckLinkname
+	}
 	switch *FlagStrictDups {
 	case 0:
 		// nothing to do
diff --git a/src/cmd/link/internal/ld/main.go b/src/cmd/link/internal/ld/main.go
index 8a67ccfb32..e6608fd791 100644
--- a/src/cmd/link/internal/ld/main.go
+++ b/src/cmd/link/internal/ld/main.go
@@ -96,6 +96,7 @@ var (
 	FlagS             = flag.Bool("s", false, "disable symbol table")
 	flag8             bool // use 64-bit addresses in symbol table
 	flagInterpreter   = flag.String("I", "", "use `linker` as ELF dynamic linker")
+	flagCheckLinkname = flag.Bool("checklinkname", false, "check linkname symbol references")
 	FlagDebugTramp    = flag.Int("debugtramp", 0, "debug trampolines")
 	FlagDebugTextSize = flag.Int("debugtextsize", 0, "debug text section max size")
 	flagDebugNosplit  = flag.Bool("debugnosplit", false, "dump nosplit call graph")
diff --git a/src/cmd/link/internal/loader/loader.go b/src/cmd/link/internal/loader/loader.go
index 53ebb53a75..0a76c1fb0c 100644
--- a/src/cmd/link/internal/loader/loader.go
+++ b/src/cmd/link/internal/loader/loader.go
@@ -292,6 +292,7 @@ type extSymPayload struct {
 const (
 	// Loader.flags
 	FlagStrictDups = 1 << iota
+	FlagCheckLinkname
 )
 
 func NewLoader(flags uint32, reporter *ErrorReporter) *Loader {
@@ -421,14 +422,6 @@ func (st *loadState) addSym(name string, ver int, r *oReader, li uint32, kind in
 	}
 
 	// Non-package (named) symbol.
-	if osym.IsLinkname() && r.DataSize(li) == 0 {
-		// This is a linknamed "var" "reference" (var x T with no data and //go:linkname x).
-		// Check if a linkname reference is allowed.
-		// Only check references (pull), not definitions (push, with non-zero size),
-		// so push is always allowed.
-		// Linkname is always a non-package reference.
-		checkLinkname(r.unit.Lib.Pkg, name)
-	}
 	// Check if it already exists.
 	oldi, existed := l.symsByName[ver][name]
 	if !existed {
@@ -2154,6 +2147,14 @@ type loadState struct {
 	l            *Loader
 	hashed64Syms map[uint64]symAndSize         // short hashed (content-addressable) symbols, keyed by content hash
 	hashedSyms   map[goobj.HashType]symAndSize // hashed (content-addressable) symbols, keyed by content hash
+
+	linknameVarRefs []linknameVarRef // linknamed var refererces
+}
+
+type linknameVarRef struct {
+	pkg  string // package of reference (not definition)
+	name string
+	sym  Sym
 }
 
 // Preload symbols of given kind from an object.
@@ -2188,6 +2189,19 @@ func (st *loadState) preloadSyms(r *oReader, kind int) {
 		}
 		gi := st.addSym(name, v, r, i, kind, osym)
 		r.syms[i] = gi
+		if kind == nonPkgDef && osym.IsLinkname() && r.DataSize(i) == 0 && strings.Contains(name, ".") {
+			// This is a linknamed "var" "reference" (var x T with no data and //go:linkname x).
+			// We want to check if a linkname reference is allowed. Here we haven't loaded all
+			// symbol definitions, so we don't yet know all the push linknames. So we add to a
+			// list and check later after all symbol defs are loaded. Linknamed vars are rare,
+			// so this list won't be long.
+			// Only check references (pull), not definitions (push, with non-zero size),
+			// so push is always allowed.
+			// This use of linkname is usually for referencing C symbols, so allow symbols
+			// with no "." in its name (not a regular Go symbol).
+			// Linkname is always a non-package reference.
+			st.linknameVarRefs = append(st.linknameVarRefs, linknameVarRef{r.unit.Lib.Pkg, name, gi})
+		}
 		if osym.Local() {
 			l.SetAttrLocal(gi, true)
 		}
@@ -2237,6 +2251,9 @@ func (l *Loader) LoadSyms(arch *sys.Arch) {
 		st.preloadSyms(r, hashedDef)
 		st.preloadSyms(r, nonPkgDef)
 	}
+	for _, vr := range st.linknameVarRefs {
+		l.checkLinkname(vr.pkg, vr.name, vr.sym)
+	}
 	l.nhashedsyms = len(st.hashed64Syms) + len(st.hashedSyms)
 	for _, r := range l.objs[goObjStart:] {
 		loadObjRefs(l, r, arch)
@@ -2252,15 +2269,15 @@ func loadObjRefs(l *Loader, r *oReader, arch *sys.Arch) {
 		osym := r.Sym(ndef + i)
 		name := osym.Name(r.Reader)
 		v := abiToVer(osym.ABI(), r.version)
+		gi := l.LookupOrCreateSym(name, v)
+		r.syms[ndef+i] = gi
 		if osym.IsLinkname() {
 			// Check if a linkname reference is allowed.
 			// Only check references (pull), not definitions (push),
 			// so push is always allowed.
 			// Linkname is always a non-package reference.
-			checkLinkname(r.unit.Lib.Pkg, name)
+			l.checkLinkname(r.unit.Lib.Pkg, name, gi)
 		}
-		r.syms[ndef+i] = l.LookupOrCreateSym(name, v)
-		gi := r.syms[ndef+i]
 		if osym.Local() {
 			l.SetAttrLocal(gi, true)
 		}
@@ -2307,30 +2324,27 @@ func abiToVer(abi uint16, localSymVersion int) int {
 
 // A list of blocked linknames. Some linknames are allowed only
 // in specific packages. This maps symbol names to allowed packages.
-// If a name is not in this map, and not with a blocked prefix (see
-// blockedLinknamePrefixes), it is allowed everywhere.
-// If a name is in this map, it is allowed only in listed packages.
+// If a name is not in this map, it is allowed iff the definition
+// has a linkname (push).
+// If a name is in this map, it is allowed only in listed packages,
+// even if it has a linknamed definition.
 var blockedLinknames = map[string][]string{
 	// coroutines
-	"runtime.coroexit":   nil,
-	"runtime.corostart":  nil,
 	"runtime.coroswitch": {"iter"},
 	"runtime.newcoro":    {"iter"},
 	// weak references
 	"internal/weak.runtime_registerWeakPointer": {"internal/weak"},
 	"internal/weak.runtime_makeStrongFromWeak":  {"internal/weak"},
-	"runtime.getOrAddWeakHandle":                nil,
 }
 
-// A list of blocked linkname prefixes (packages).
-var blockedLinknamePrefixes = []string{
-	"internal/weak.",
-	"internal/concurrent.",
-}
+// check if a linkname reference to symbol s from pkg is allowed
+func (l *Loader) checkLinkname(pkg, name string, s Sym) {
+	if l.flags&FlagCheckLinkname == 0 {
+		return
+	}
 
-func checkLinkname(pkg, name string) {
 	error := func() {
-		log.Fatalf("linkname or assembly reference of %s is not allowed in package %s", name, pkg)
+		log.Fatalf("%s: invalid reference to %s", pkg, name)
 	}
 	pkgs, ok := blockedLinknames[name]
 	if ok {
@@ -2341,11 +2355,26 @@ func checkLinkname(pkg, name string) {
 		}
 		error()
 	}
-	for _, p := range blockedLinknamePrefixes {
-		if strings.HasPrefix(name, p) {
-			error()
-		}
+	r, li := l.toLocal(s)
+	if r == l.extReader { // referencing external symbol is okay
+		return
+	}
+	if !r.Std() { // For now, only check for symbols defined in std
+		return
+	}
+	if r.unit.Lib.Pkg == pkg { // assembly reference from same package
+		return
+	}
+	osym := r.Sym(li)
+	if osym.IsLinkname() || osym.ABIWrapper() {
+		// Allow if the def has a linkname (push).
+		// ABI wrapper usually wraps an assembly symbol, a linknamed symbol,
+		// or an external symbol, or provide access of a Go symbol to assembly.
+		// For now, allow ABI wrappers.
+		// TODO: check the wrapped symbol?
+		return
 	}
+	error()
 }
 
 // TopLevelSym tests a symbol (by name and kind) to determine whether
diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go
index 3abec64c5d..1ce484fe61 100644
--- a/src/cmd/link/link_test.go
+++ b/src/cmd/link/link_test.go
@@ -1416,7 +1416,7 @@ func TestRandLayout(t *testing.T) {
 	}
 }
 
-func TestBlockedLinkname(t *testing.T) {
+func TestCheckLinkname(t *testing.T) {
 	// Test that code containing blocked linknames does not build.
 	testenv.MustHaveGoBuild(t)
 	t.Parallel()
@@ -1433,10 +1433,13 @@ func TestBlockedLinkname(t *testing.T) {
 		{"push.go", true},
 		// pull linkname of blocked symbol is not ok
 		{"coro.go", false},
-		{"weak.go", false},
 		{"coro_var.go", false},
 		// assembly reference is not ok
 		{"coro_asm", false},
+		// pull-only linkname is not ok
+		{"coro2.go", false},
+		// legacy bad linkname is ok, for now
+		{"fastrand.go", true},
 	}
 	for _, test := range tests {
 		test := test
@@ -1444,7 +1447,7 @@ func TestBlockedLinkname(t *testing.T) {
 			t.Parallel()
 			src := filepath.Join("testdata", "linkname", test.src)
 			exe := filepath.Join(tmpdir, test.src+".exe")
-			cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-o", exe, src)
+			cmd := testenv.Command(t, testenv.GoToolPath(t), "build", "-ldflags=-checklinkname=1", "-o", exe, src)
 			out, err := cmd.CombinedOutput()
 			if test.ok && err != nil {
 				t.Errorf("build failed unexpectedly: %v:\n%s", err, out)
diff --git a/src/cmd/link/testdata/linkname/coro2.go b/src/cmd/link/testdata/linkname/coro2.go
new file mode 100644
index 0000000000..ae47147670
--- /dev/null
+++ b/src/cmd/link/testdata/linkname/coro2.go
@@ -0,0 +1,17 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Linkname corostart is not allowed, as it doesn't have
+// a linknamed definition.
+
+package main
+
+import _ "unsafe"
+
+//go:linkname corostart runtime.corostart
+func corostart()
+
+func main() {
+	corostart()
+}
diff --git a/src/cmd/link/testdata/linkname/fastrand.go b/src/cmd/link/testdata/linkname/fastrand.go
new file mode 100644
index 0000000000..ce51e2a7f3
--- /dev/null
+++ b/src/cmd/link/testdata/linkname/fastrand.go
@@ -0,0 +1,18 @@
+// Copyright 2024 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Linkname fastrand is allowed _for now_, as it has a
+// linknamed definition, for legacy reason.
+// NOTE: this may not be allowed in the future. Don't do this!
+
+package main
+
+import _ "unsafe"
+
+//go:linkname fastrand runtime.fastrand
+func fastrand() uint32
+
+func main() {
+	println(fastrand())
+}
diff --git a/src/cmd/link/testdata/linkname/weak.go b/src/cmd/link/testdata/linkname/weak.go
deleted file mode 100644
index 2bf0fbcbab..0000000000
--- a/src/cmd/link/testdata/linkname/weak.go
+++ /dev/null
@@ -1,22 +0,0 @@
-// Copyright 2024 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Linkname generic functions in internal/weak is not
-// allowed; legitimate instantiation is ok.
-
-package main
-
-import (
-	"unique"
-	"unsafe"
-)
-
-//go:linkname weakMake internal/weak.Make[string]
-func weakMake(string) unsafe.Pointer
-
-func main() {
-	h := unique.Make("xxx")
-	println(h.Value())
-	weakMake("xxx")
-}