]> Cypherpunks repositories - gostls13.git/commitdiff
Revert: "cmd/link: add option to enable full RELRO for ELF"
authorThan McIntosh <thanm@google.com>
Wed, 13 Mar 2024 17:46:29 +0000 (17:46 +0000)
committerThan McIntosh <thanm@google.com>
Thu, 14 Mar 2024 14:41:04 +0000 (14:41 +0000)
This reverts https://go.dev/cl/c/go/+/473495.

Reason for revert: breaks some Google-internal tests.

This revert will be temporary until we can gather more info on the
nature of the failures and hopefully develop an upstream test case,
etc.

Updates #45681.

Change-Id: Ib628ddc53bc5489e4f76c0f4ad809b75e899102c
Reviewed-on: https://go-review.googlesource.com/c/go/+/571415
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
src/cmd/go/internal/work/security.go
src/cmd/go/internal/work/security_test.go
src/cmd/link/doc.go
src/cmd/link/internal/ld/elf.go
src/cmd/link/internal/ld/elf_test.go
src/cmd/link/internal/ld/lib.go
src/cmd/link/internal/ld/main.go
src/internal/testenv/testenv.go

index 3289276e7749cb001ce6fbe19752e303b689070e..88504be6cd6b4123a04c8f34381ff53b95afd56c 100644 (file)
@@ -210,7 +210,8 @@ var validLinkerFlags = []*lazyregexp.Regexp{
        re(`-Wl,-?-unresolved-symbols=[^,]+`),
        re(`-Wl,--(no-)?warn-([^,]+)`),
        re(`-Wl,-?-wrap[=,][^,@\-][^,]*`),
-       re(`-Wl(,-z,(relro|now|(no)?execstack))+`),
+       re(`-Wl,-z,(no)?execstack`),
+       re(`-Wl,-z,relro`),
 
        re(`[a-zA-Z0-9_/].*\.(a|o|obj|dll|dylib|so|tbd)`), // direct linker inputs: x.o or libfoo.so (but not -foo.o or @foo.o)
        re(`\./.*\.(a|o|obj|dll|dylib|so|tbd)`),
index a4c055670ae3807a4c89d8db6a157b1c2da68fd4..c05ba7b9a472e3dbab12c5c70b5d4bd50c865620 100644 (file)
@@ -167,10 +167,6 @@ var goodLinkerFlags = [][]string{
        {"-Wl,-framework", "-Wl,Chocolate"},
        {"-Wl,-framework,Chocolate"},
        {"-Wl,-unresolved-symbols=ignore-all"},
-       {"-Wl,-z,relro"},
-       {"-Wl,-z,relro,-z,now"},
-       {"-Wl,-z,now"},
-       {"-Wl,-z,noexecstack"},
        {"libcgotbdtest.tbd"},
        {"./libcgotbdtest.tbd"},
 }
index bd620f987831f7f435b2be4325242edafea3c843..b0f2700ac1d2536dcf2463f4d6aeca916e84e135 100644 (file)
@@ -47,8 +47,6 @@ Flags:
                Link with C/C++ address sanitizer support.
        -aslr
                Enable ASLR for buildmode=c-shared on windows (default true).
-       -bindnow
-               Mark a dynamically linked ELF object for immediate function binding (default false).
        -buildid id
                Record id as Go toolchain build id.
        -buildmode mode
index 746f359b68299f3f5635efd86fe88e71e7402ef9..71e2873a1d7a17753b0a73d459d64b55fbb2ec39 100644 (file)
@@ -1056,17 +1056,11 @@ func elfdynhash(ctxt *Link) {
        }
 
        s = ldr.CreateSymForUpdate(".dynamic", 0)
-
-       var dtFlags1 elf.DynFlag1
-       if *flagBindNow {
-               dtFlags1 |= elf.DF_1_NOW
-               Elfwritedynent(ctxt.Arch, s, elf.DT_FLAGS, uint64(elf.DF_BIND_NOW))
-       }
        if ctxt.BuildMode == BuildModePIE {
-               dtFlags1 |= elf.DF_1_PIE
+               // https://github.com/bminor/glibc/blob/895ef79e04a953cac1493863bcae29ad85657ee1/elf/elf.h#L986
+               const DTFLAGS_1_PIE = 0x08000000
+               Elfwritedynent(ctxt.Arch, s, elf.DT_FLAGS_1, uint64(DTFLAGS_1_PIE))
        }
-       Elfwritedynent(ctxt.Arch, s, elf.DT_FLAGS_1, uint64(dtFlags1))
-
        elfverneed = nfile
        if elfverneed != 0 {
                elfWriteDynEntSym(ctxt, s, elf.DT_VERNEED, gnuVersionR.Sym())
@@ -1113,7 +1107,6 @@ func elfphload(seg *sym.Segment) *ElfPhdr {
 func elfphrelro(seg *sym.Segment) {
        ph := newElfPhdr()
        ph.Type = elf.PT_GNU_RELRO
-       ph.Flags = elf.PF_R
        ph.Vaddr = seg.Vaddr
        ph.Paddr = seg.Vaddr
        ph.Memsz = seg.Length
@@ -1563,11 +1556,7 @@ func (ctxt *Link) doelf() {
 
                /* global offset table */
                got := ldr.CreateSymForUpdate(".got", 0)
-               if ctxt.UseRelro() {
-                       got.SetType(sym.SRODATARELRO)
-               } else {
-                       got.SetType(sym.SELFGOT) // writable
-               }
+               got.SetType(sym.SELFGOT) // writable
 
                /* ppc64 glink resolver */
                if ctxt.IsPPC64() {
@@ -1580,11 +1569,7 @@ func (ctxt *Link) doelf() {
                hash.SetType(sym.SELFROSECT)
 
                gotplt := ldr.CreateSymForUpdate(".got.plt", 0)
-               if ctxt.UseRelro() && *flagBindNow {
-                       gotplt.SetType(sym.SRODATARELRO)
-               } else {
-                       gotplt.SetType(sym.SELFSECT) // writable
-               }
+               gotplt.SetType(sym.SELFSECT) // writable
 
                plt := ldr.CreateSymForUpdate(".plt", 0)
                if ctxt.IsPPC64() {
@@ -1606,12 +1591,9 @@ func (ctxt *Link) doelf() {
 
                /* define dynamic elf table */
                dynamic := ldr.CreateSymForUpdate(".dynamic", 0)
-               switch {
-               case thearch.ELF.DynamicReadOnly:
+               if thearch.ELF.DynamicReadOnly {
                        dynamic.SetType(sym.SELFROSECT)
-               case ctxt.UseRelro():
-                       dynamic.SetType(sym.SRODATARELRO)
-               default:
+               } else {
                        dynamic.SetType(sym.SELFSECT)
                }
 
index 16bf4039b1f3548f767ea3eefbbf863dfaba3f0c..e535af6a1c2e6d4db0f5e23f8ab4b4091e8a94ca 100644 (file)
@@ -8,7 +8,6 @@ package ld
 
 import (
        "debug/elf"
-       "fmt"
        "internal/testenv"
        "os"
        "path/filepath"
@@ -183,152 +182,3 @@ func main() {
                }
        }
 }
-
-func TestElfBindNow(t *testing.T) {
-       t.Parallel()
-       testenv.MustHaveGoBuild(t)
-
-       const (
-               prog = `package main; func main() {}`
-               // with default buildmode code compiles in a statically linked binary, hence CGO
-               progC = `package main; import "C"; func main() {}`
-       )
-
-       tests := []struct {
-               name                 string
-               args                 []string
-               prog                 string
-               mustHaveBuildModePIE bool
-               mustHaveCGO          bool
-               mustInternalLink     bool
-               wantDfBindNow        bool
-               wantDf1Now           bool
-               wantDf1Pie           bool
-       }{
-               {name: "default", prog: prog},
-               {
-                       name:                 "pie-linkmode-internal",
-                       args:                 []string{"-buildmode=pie", "-ldflags", "-linkmode=internal"},
-                       prog:                 prog,
-                       mustHaveBuildModePIE: true,
-                       mustInternalLink:     true,
-                       wantDf1Pie:           true,
-               },
-               {
-                       name:             "bindnow-linkmode-internal",
-                       args:             []string{"-ldflags", "-bindnow -linkmode=internal"},
-                       prog:             progC,
-                       mustHaveCGO:      true,
-                       mustInternalLink: true,
-                       wantDfBindNow:    true,
-                       wantDf1Now:       true,
-               },
-               {
-                       name:                 "bindnow-pie-linkmode-internal",
-                       args:                 []string{"-buildmode=pie", "-ldflags", "-bindnow -linkmode=internal"},
-                       prog:                 prog,
-                       mustHaveBuildModePIE: true,
-                       mustInternalLink:     true,
-                       wantDfBindNow:        true,
-                       wantDf1Now:           true,
-                       wantDf1Pie:           true,
-               },
-               {
-                       name:                 "bindnow-pie-linkmode-external",
-                       args:                 []string{"-buildmode=pie", "-ldflags", "-bindnow -linkmode=external"},
-                       prog:                 prog,
-                       mustHaveBuildModePIE: true,
-                       mustHaveCGO:          true,
-                       wantDfBindNow:        true,
-                       wantDf1Now:           true,
-                       wantDf1Pie:           true,
-               },
-       }
-
-       gotDynFlag := func(flags []uint64, dynFlag uint64) bool {
-               for _, flag := range flags {
-                       if gotFlag := dynFlag&flag != 0; gotFlag {
-                               return true
-                       }
-               }
-
-               return false
-       }
-
-       for _, test := range tests {
-               t.Run(test.name, func(t *testing.T) {
-                       if test.mustInternalLink {
-                               testenv.MustInternalLink(t, test.mustHaveCGO)
-                       }
-                       if test.mustHaveCGO {
-                               testenv.MustHaveCGO(t)
-                       }
-                       if test.mustHaveBuildModePIE {
-                               testenv.MustHaveBuildMode(t, "pie")
-                       }
-                       if test.mustHaveBuildModePIE && test.mustInternalLink {
-                               testenv.MustInternalLinkPIE(t)
-                       }
-
-                       var (
-                               dir     = t.TempDir()
-                               src     = filepath.Join(dir, fmt.Sprintf("elf_%s.go", test.name))
-                               binFile = filepath.Join(dir, test.name)
-                       )
-
-                       if err := os.WriteFile(src, []byte(test.prog), 0666); err != nil {
-                               t.Fatal(err)
-                       }
-
-                       cmdArgs := append([]string{"build", "-o", binFile}, append(test.args, src)...)
-                       cmd := testenv.Command(t, testenv.GoToolPath(t), cmdArgs...)
-
-                       if out, err := cmd.CombinedOutput(); err != nil {
-                               t.Fatalf("failed to build %v: %v:\n%s", cmd.Args, err, out)
-                       }
-
-                       fi, err := os.Open(binFile)
-                       if err != nil {
-                               t.Fatalf("failed to open built file: %v", err)
-                       }
-                       defer fi.Close()
-
-                       elfFile, err := elf.NewFile(fi)
-                       if err != nil {
-                               t.Skip("The system may not support ELF, skipped.")
-                       }
-                       defer elfFile.Close()
-
-                       flags, err := elfFile.DynValue(elf.DT_FLAGS)
-                       if err != nil {
-                               t.Fatalf("failed to get DT_FLAGS: %v", err)
-                       }
-
-                       flags1, err := elfFile.DynValue(elf.DT_FLAGS_1)
-                       if err != nil {
-                               t.Fatalf("failed to get DT_FLAGS_1: %v", err)
-                       }
-
-                       gotDfBindNow := gotDynFlag(flags, uint64(elf.DF_BIND_NOW))
-                       gotDf1Now := gotDynFlag(flags1, uint64(elf.DF_1_NOW))
-
-                       bindNowFlagsMatch := gotDfBindNow == test.wantDfBindNow && gotDf1Now == test.wantDf1Now
-
-                       // some external linkers may set one of the two flags but not both.
-                       if !test.mustInternalLink {
-                               bindNowFlagsMatch = gotDfBindNow == test.wantDfBindNow || gotDf1Now == test.wantDf1Now
-                       }
-
-                       if !bindNowFlagsMatch {
-                               t.Fatalf("Dynamic flags mismatch:\n"+
-                                       "DT_FLAGS BIND_NOW      got: %v,        want: %v\n"+
-                                       "DT_FLAGS_1 DF_1_NOW    got: %v,        want: %v",
-                                       gotDfBindNow, test.wantDfBindNow, gotDf1Now, test.wantDf1Now)
-                       }
-
-                       if gotDf1Pie := gotDynFlag(flags1, uint64(elf.DF_1_PIE)); gotDf1Pie != test.wantDf1Pie {
-                               t.Fatalf("DT_FLAGS_1 DF_1_PIE got: %v, want: %v", gotDf1Pie, test.wantDf1Pie)
-                       }
-               })
-       }
-}
index c68da4070b4762266920bc2bc428179c1b3d8de8..ae19896077edfef2e3a02d4c12f2704dfc2b0138 100644 (file)
@@ -1604,16 +1604,12 @@ func (ctxt *Link) hostlink() {
        }
 
        var altLinker string
-       if ctxt.IsELF && (ctxt.DynlinkingGo() || *flagBindNow) {
-               // For ELF targets, when producing dynamically linked Go code
-               // or when immediate binding is explicitly requested,
-               // we force all symbol resolution to be done at program startup
+       if ctxt.IsELF && ctxt.DynlinkingGo() {
+               // We force all symbol resolution to be done at program startup
                // because lazy PLT resolution can use large amounts of stack at
                // times we cannot allow it to do so.
                argv = append(argv, "-Wl,-z,now")
-       }
 
-       if ctxt.IsELF && ctxt.DynlinkingGo() {
                // Do not let the host linker generate COPY relocations. These
                // can move symbols out of sections that rely on stable offsets
                // from the beginning of the section (like sym.STYPE).
index 13077668e7be5a07678b152399efb3b8a62143d5..a0cc52a02922640f17f04e8a5f6b19a8a7569fb6 100644 (file)
@@ -63,7 +63,6 @@ func init() {
 // Flags used by the linker. The exported flags are used by the architecture-specific packages.
 var (
        flagBuildid = flag.String("buildid", "", "record `id` as Go toolchain build id")
-       flagBindNow = flag.Bool("bindnow", false, "mark a dynamically linked ELF object for immediate function binding")
 
        flagOutfile    = flag.String("o", "", "write output to `file`")
        flagPluginPath = flag.String("pluginpath", "", "full path name for plugin")
index 3b9d2fd1e9194ad5add63b421250376b743cb84e..f767ac590ca53c39c2f442c2e35e56faff49eefd 100644 (file)
@@ -369,15 +369,6 @@ func MustInternalLink(t testing.TB, withCgo bool) {
        }
 }
 
-// MustInternalLinkPIE checks whether the current system can link PIE binary using
-// internal linking.
-// If not, MustInternalLinkPIE calls t.Skip with an explanation.
-func MustInternalLinkPIE(t testing.TB) {
-       if !platform.InternalLinkPIESupported(runtime.GOOS, runtime.GOARCH) {
-               t.Skipf("skipping test: internal linking for buildmode=pie on %s/%s is not supported", runtime.GOOS, runtime.GOARCH)
-       }
-}
-
 // MustHaveBuildMode reports whether the current system can build programs in
 // the given build mode.
 // If not, MustHaveBuildMode calls t.Skip with an explanation.