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

cmd/link: define ELF .note section on FreeBSD #48164

Open
igalic opened this issue Sep 2, 2021 · 12 comments
Open

cmd/link: define ELF .note section on FreeBSD #48164

igalic opened this issue Sep 2, 2021 · 12 comments

Comments

@igalic
Copy link

@igalic igalic commented Sep 2, 2021

What version of Go are you using (go version)?

$ go version
go version go1.17 freebsd/amd64

Does this issue reproduce with the latest release?

yes (this is the latest version)

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/meena/.cache/go-build"
GOENV="/home/meena/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOINSECURE=""
GOMODCACHE="/home/meena/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="freebsd"
GOPATH="/home/meena/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/freebsd_amd64"
GOVCS=""
GOVERSION="go1.17"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1853362343=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Compile a simple program on FreeBSD 13:

package main

import (
        "fmt"
)

func main() {
        fmt.Println("Hello, World!")
}
meena@beasdix ~/s/g/hello> go build -v hello.go
meena@beasdix ~/s/g/hello> 

What did you expect to see?

meena@beasdix ~/s/g/hello> elfdump -n hello

note (.note.tag):
        FreeBSD 1300139 (NT_FREEBSD_ABI_TAG)
        FreeBSD 0 (NT_FREEBSD_FEATURE_CTL)
        FreeBSD 0 (NT_FREEBSD_NOINIT_TAG)
meena@beasdix ~/s/g/hello> 

What did you see instead?

meena@beasdix ~/s/g/hello> elfdump -n hello
meena@beasdix ~/s/g/hello> 

Analogous headers for OpenBSD, NetBSD and Linux already exist.
Ours are declared here: https://github.com/freebsd/freebsd-src/blob/main/sys/sys/elf_common.h#L788-L792

@igalic igalic changed the title cmd/link cmd/link: define ELF .note section on FreeBSD Sep 2, 2021
@randall77
Copy link
Contributor

@randall77 randall77 commented Sep 2, 2021

What do these notes do? Why would we want to add them?

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Sep 2, 2021

Is there any document for those? What do they mean? Thanks.

@igalic
Copy link
Author

@igalic igalic commented Sep 2, 2021

This issue is related to #48112

NT_FREEBSD_FEATURE_CTL in particular is used to mark a binary as incompatible with certain security measures, via the elfctl(1) tool:

meena@beasdix ~> elfctl (which ls)
File '/bin/ls' features:
noaslr          'Disable ASLR' is unset.
noprotmax       'Disable implicit PROT_MAX' is unset.
nostackgap      'Disable stack gap' is unset.
wxneeded        'Requires W+X mappings' is unset.
la48            'amd64: Limit user VA to 48bit' is unset.
noaslrstkgap    'Disable ASLR stack gap' is unset.
meena@beasdix ~> 

I hope @emaste can share more insight on the other notes.

@randall77
Copy link
Contributor

@randall77 randall77 commented Sep 2, 2021

Is there a reason why we would want to set any of these?

@jrtc27
Copy link
Contributor

@jrtc27 jrtc27 commented Sep 2, 2021

NT_FREEBSD_ABI_TAG indicates what version of FreeBSD the binary was compiled for and does two things: (a) when there are breaking changes to kernel behaviour that can sometimes be gated behind binaries saying they were built for a version that already had that behaviour, so existing binaries don't break (b) there is a sysctl you can enable to stop you running binaries built for newer kernels than the one you are running. Most things work without it but you may see strange behaviour in edge cases if you don't put an appropriate version there. (Also an honorary mention for RISC-V: RISC-V binaries end up with NT_FREEBSD_ABI_TAG being the only FreeBSD branding for statically-linked binaries, so without that you'll get ENOEXEC for them on FreeBSD, and for dynamically-linked binaries ldd will give an error that it's not a FreeBSD binary. I go back and forth on whether the lack of EI_OSABI being ELFOSABI_FREEBSD (a field which gets abused for multiple purposes so there's an argument to be made for relying on extensible ELF notes and stopping conflating what that field means) is a good idea, but it's what happens today.)

NT_FREEBSD_NOINIT_TAG tells the run-time linker (well, its presence or lack thereof, the value is unused) whether or not the CRT files call _init, .ctors and .init_array or RTLD is expected to; the old GNU-derived files called them themselves, the new from-scratch BSD-licensed replacements leave it up to RTLD to do. Whether or not you define this should be based on whether Go's runtime calls these functions or expects the run-time linker to (though if you only do static linking it's not consulted, and if you never try to make use of those various constructor-like features then whether it exists doesn't matter).

NT_FREEBSD_FEATURE_CTL tells the kernel whether or not you are opting out of various security features. If it doesn't exist then you don't opt out of anything and there is no way to change that; if you add it but set it to 0 then elfctl can later change the value (e.g. a user discovers that a binary breaks with a certain hardening feature applied), but it cannot add the note if it doesn't exist in the first place.

So you must define NT_FREEBSD_ABI_TAG and NT_FREEBSD_FEATURE_CTL appropriately, and whether or not you should define NT_FREEBSD_NOINIT_TAG depends on what exactly Go does and supports. Things generally work without them but sooner or later something somewhere will break as a result of playing it fast and loose.

Of course, the best thing to do is use the system's CRT files rather than doing everything from scratch, as it's only a backwards-compatible interface so existing binaries work, not a stable one that we don't guarantee won't change (e.g. someone could technically add a new note tomorrow that's required for FreeBSD 14 binaries, even if it's highly unlikely).

@randall77
Copy link
Contributor

@randall77 randall77 commented Sep 2, 2021

NT_FREEBSD_ABI_TAG - we could set this to the minimum freebsd version we support. Might be kind of a pain to maintain though (or maybe if we bump to a higher minimum, and use a syscall now available with that new minimum, if we forgot to bump this tag then the kernel wouldn't let us call that new syscall?)

NT_FREEBSD_NOINIT_TAG - we don't call any of these when internally linking, so I don't think we need this one.

NT_FREEBSD_FEATURE_CTL - I don't think we need to opt out of any security features (at least, the ones @igalic listed) when internally linking either, although I guess it is possible that someone would want to modify a binary post-link, for example to disable ASLR in order to facilitate debugging. So we need to put an all-empty tag in the binary so someone could modify it?

@jrtc27
Copy link
Contributor

@jrtc27 jrtc27 commented Sep 2, 2021

NT_FREEBSD_ABI_TAG - we could set this to the minimum freebsd version we support. Might be kind of a pain to maintain though (or maybe if we bump to a higher minimum, and use a syscall now available with that new minimum, if we forgot to bump this tag then the kernel wouldn't let us call that new syscall?)

It's really only used in a handful of places: a few sysctls, mmap gained a couple of checks for nonsense arguments like len == 0, the shutdown(2) socket syscall started to return ENOTCONN when issued on a socket that wasn't connected, sigwait(2) started to return EINTR rather than looping, and whether you get a SIGBUS or SIGSEGV for page faults due to permissions, that kind of subtle thing that almost never matters until it does. My guess is the right thing to do is set the version to whatever you assume the version of FreeBSD to be, and you can dynamically detect support for newer syscalls if you want (though watch out for SIGSYS), though don't quote me on that because I don't know what we guarantee, if anything (the CRT files are supposed to be kept in sync with your system header files, so it's not normally possible to know about new syscalls that your ABI note doesn't say you should know about unless you hard-code the number in your C source).

NT_FREEBSD_NOINIT_TAG - we don't call any of these when internally linking, so I don't think we need this one.

The tag is for precisely when you don't call those functions. If those functions/tables of functions never exist then it doesn't matter, and if you're statically linked then it doesn't matter either, but not calling them is not a reason to not include the note, quite the opposite.

NT_FREEBSD_FEATURE_CTL - I don't think we need to opt out of any security features (at least, the ones @igalic listed) when internally linking either, although I guess it is possible that someone would want to modify a binary post-link, for example to disable ASLR in order to facilitate debugging. So we need to put an all-empty tag in the binary so someone could modify it?

Yeah. This is also what the default CRT file does, and if a given binary needs something else then it's supposed to invoke elfctl during its build. It's just a placeholder to make elfctl work (without having to pick apart and reassemble the ELF file, which can be rather fraught depending on the architecture).

@igalic
Copy link
Author

@igalic igalic commented Sep 8, 2021

This issue is still saying WaitingForInfo.
What more info can we provide?

@randall77
Copy link
Contributor

@randall77 randall77 commented Sep 8, 2021

I think we might have the info required. Presumably the format is similar to the other BSDs?

@igalic
Copy link
Author

@igalic igalic commented Sep 8, 2021

@randall77 from what I've seen in src/cmd/link/internal/ld/elf.go, i would say: yes.

but, and as I'm seeing in the NetBSD section:

// NetBSD Signature (as per sys/exec_elf.h)
const (
        ELF_NOTE_NETBSD_NAMESZ  = 7
        ELF_NOTE_NETBSD_DESCSZ  = 4
        ELF_NOTE_NETBSD_TAG     = 1
        ELF_NOTE_NETBSD_VERSION = 700000000 /* NetBSD 7.0 */
)

i would "simply" pull which ever version you're currently compiling on.
I think that's what @jrtc27 was trying to hint at, but maybe i misunderstood her.

@randall77
Copy link
Contributor

@randall77 randall77 commented Sep 8, 2021

We try not to make the build dependent on the build machine. That way the behavior of cross-compilation and native compilation don't diverge. Especially for this case, when we're internally linking.

@emaste
Copy link

@emaste emaste commented Sep 8, 2021

From lib/csu/common/crtbrand.S:

/*
 * Special ".note.tag" entry specifying the ABI version.  See
 * http://www.netbsd.org/Documentation/kernel/elf-notes.html
 * for more information.
 */

        .section .note.tag,"aG",%note,.freebsd.noteG,comdat
        .p2align        2
        .4byte          2f-1f
        .4byte          4f-3f
        .4byte          NT_FREEBSD_ABI_TAG
1:      .asciz          NOTE_FREEBSD_VENDOR
2:      .p2align        2
3:      .4byte          __FreeBSD_version

For example, data from /bin/ls on my laptop:

Size    Data            Description
4       0x00000008      namesz
4       0x00000004      descsz
4       0x00000001      type (NT_FREEBSD_ABI_TAG)
8       FreeBSD\0       vendor
4       1300136         data

(1300136 is the value from the FreeBSD 13 branch from whenever I built kernel/world on my laptop)

There is a list of __FreeBSD_version values at https://docs.freebsd.org/en/books/porters-handbook/versions/
FreeBSD 10.x is out of support, but for FreeBSD 10.4 the __FreeBSD_version is 1004500.

sys/sys/param.h has constants that the kernel/rtld compare against:

#if defined(_KERNEL) || defined(IN_RTLD)
#define P_OSREL_SIGWAIT                 700000
#define P_OSREL_SIGSEGV                 700004
#define P_OSREL_MAP_ANON                800104
#define P_OSREL_MAP_FSTRICT             1100036
#define P_OSREL_SHUTDOWN_ENOTCONN       1100077
#define P_OSREL_MAP_GUARD               1200035
#define P_OSREL_WRFSBASE                1200041
#define P_OSREL_CK_CYLGRP               1200046
#define P_OSREL_VMTOTAL64               1200054
#define P_OSREL_CK_SUPERBLOCK           1300000
#define P_OSREL_CK_INODE                1300005
#define P_OSREL_POWERPC_NEW_AUX_ARGS    1300070

#define P_OSREL_MAJOR(x)                ((x) / 100000)
#endif

If we add this NT_FREEBSD_ABI_TAG note with value 1004500 then the P_OSREL_SIGWAIT, P_OSREL_SIGSEGV, P_OSREL_MAP_ANON checks will become true.

Let me see what this will change:

        if (error) {
                /*
                 * sigwait() function shall not return EINTR, but
                 * the syscall does.  Non-ancient libc provides the
                 * wrapper which hides EINTR.  Otherwise, EINTR return
                 * is used by libthr to handle required cancellation
                 * point in the sigwait().
                 */
                if (error == EINTR && td->td_proc->p_osrel < P_OSREL_SIGWAIT)
                        return (ERESTART);
                td->td_retval[0] = error;
                return (0);
        }

So, caller must be prepared to handle EINTR from sigwait syscall

                        if (prot_fault_translation == 0) {
                                /*
                                 * Autodetect.  This check also covers
                                 * the images without the ABI-tag ELF
                                 * note.
                                 */
                                if (SV_CURPROC_ABI() == SV_ABI_FREEBSD &&
                                    curproc->p_osrel >= P_OSREL_SIGSEGV) {
                                        *signo = SIGSEGV;
                                        *ucode = SEGV_ACCERR;
                                } else {
                                        *signo = SIGBUS;
                                        *ucode = UCODE_PAGEFLT;
                                }
                        } else if (prot_fault_translation == 1) {
                                /* Always compat mode. */
                                *signo = SIGBUS;
                                *ucode = UCODE_PAGEFLT;
                        } else {
                                /* Always SIGSEGV mode. */
                                *signo = SIGSEGV;
                                *ucode = SEGV_ACCERR;
                        }
                        break;

From freebsd/freebsd-src@df08823

    Change the delivered signal for accesses to mapped area after the
    backing object was truncated.  According to POSIX description for
    mmap(2):
       The system shall always zero-fill any partial page at the end of an
       object. Further, the system shall never write out any modified
       portions of the last page of an object which are beyond its
       end. References within the address range starting at pa and
       continuing for len bytes to whole pages following the end of an
       object shall result in delivery of a SIGBUS signal.

P_OSREL_MAP_ANON is for some a.out special case, not interesting in this context.

i would "simply" pull which ever version you're currently compiling on.

If we're not actually using the target's headers then this isn't what we want -- we must use the version corresponding to Go's definitions of kernel data-structures and expectation of kernel behaviour.

While Go claims to support FreeBSD 10.x and later I think we should use 1004500. Before too long we should bump the minimum supported version, and when doing so make sure to check and if necessary update for newly-activated osrel behaviour. If we go to FreeBSD 11.x as a minimum P_OSREL_MAP_FSTRICT and P_OSREL_SHUTDOWN_ENOTCONN will be true.

The former relates to invalid combination of mmap(2) flags and shouldn't be of concern, while the latter means shutdown(2) will start returning ENOTCONN if the socket is not connected.

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

Successfully merging a pull request may close this issue.

None yet
6 participants