Summary: | Linux static golang binaries crash at startup | ||
---|---|---|---|
Product: | Base System | Reporter: | Edward Tomasz Napierala <trasz> |
Component: | kern | Assignee: | Dmitry Chagin <dchagin> |
Status: | Closed FIXED | ||
Severity: | Affects Only Me | CC: | cem, chardon.frederic, chuck, dchagin, emaste, yanko.yankulov |
Priority: | --- | ||
Version: | CURRENT | ||
Hardware: | Any | ||
OS: | Any |
Description
Edward Tomasz Napierala
2018-01-12 12:07:27 UTC
FWIW, reverting r313993 doesn't seem to fix it. Also FWIW, reverting r313993 seems to break dynamically linked golang binaries. What is the Linux code for runtime.settls? And what is the value of %fs after the crash? Can you share the Linux gofmt binary somewhere (freefall)? (In reply to Edward Tomasz Napierala from comment #1) I think r313993 did (sort of) introduce this bug. I'm curious why reverting it does not fix the issue. I think one problem may be the lack of set_pcb_flags(pcb, PCB_FULL_IRET) in linux_set_cloned_tls(). Here's the problem: AMD64_SET_FSBASE expects a pointer to a pointer: case AMD64_SET_FSBASE: error = copyin(uap->parms, &a64base, sizeof(a64base)); if (!error) { if (a64base < VM_MAXUSER_ADDRESS) { set_pcb_flags(pcb, PCB_FULL_IRET); pcb->pcb_fsbase = a64base; td->td_frame->tf_fs = _ufssel; } else error = EINVAL; } break; linux_arch_prctl() after r313993 is just passing in the pointer value itself: case LINUX_ARCH_SET_FS: bsd_args.op = AMD64_SET_FSBASE; bsd_args.parms = (void *)args->addr; error = sysarch(td, &bsd_args); Previously, it would set the value args->addr directly: case LINUX_ARCH_SET_FS: error = linux_set_cloned_tls(td, (void *)args->addr); ... linux_set_cloned_tls(struct thread *td, void *desc) { ... pcb = td->td_pcb; pcb->pcb_fsbase = (register_t)desc; Please try this patch: --- a/sys/amd64/linux/linux_machdep.c +++ b/sys/amd64/linux/linux_machdep.c @@ -234,14 +234,14 @@ linux_arch_prctl(struct thread *td, struct linux_arch_prctl_args *args) switch (args->code) { case LINUX_ARCH_SET_GS: bsd_args.op = AMD64_SET_GSBASE; - bsd_args.parms = (void *)args->addr; + bsd_args.parms = (void *)&args->addr; error = sysarch(td, &bsd_args); if (error == EINVAL) error = EPERM; break; case LINUX_ARCH_SET_FS: bsd_args.op = AMD64_SET_FSBASE; - bsd_args.parms = (void *)args->addr; + bsd_args.parms = (void *)&args->addr; error = sysarch(td, &bsd_args); if (error == EINVAL) error = EPERM; I would also consider changing linux_set_cloned_tls to match sysarch() AMD64_SET_FSBASE: @@ -271,6 +271,7 @@ linux_set_cloned_tls(struct thread *td, void *desc) return (EPERM); pcb = td->td_pcb; + set_pcb_flags(pcb, PCB_FULL_IRET); pcb->pcb_fsbase = (register_t)desc; td->td_frame->tf_fs = _ufssel; Or better yet, just invoking sysarch() as well: @@ -265,14 +265,13 @@ linux_arch_prctl(struct thread *td, struct linux_arch_prctl_args *args) int linux_set_cloned_tls(struct thread *td, void *desc) { - struct pcb *pcb; - - if ((uint64_t)desc >= VM_MAXUSER_ADDRESS) - return (EPERM); - - pcb = td->td_pcb; - pcb->pcb_fsbase = (register_t)desc; - td->td_frame->tf_fs = _ufssel; + struct sysarch_args bsd_args; + int error; - return (0); + bsd_args.op = AMD64_SET_FSBASE; + bsd_args.parms = (void *)&desc; + error = sysarch(td, &bsd_args); + if (error == EINVAL) + error = EPERM; + return (error); } The first chunks (passing pointers to pointers) breaks the dynamically linked binary: % ./go cannot set up thread-local storage: cannot set %fs base address for thread-local storage; ktrace looks like this: 64990 go CALL linux_arch_prctl(0x1002,0x800ac31c0) 64990 go RET linux_arch_prctl -1 errno -14 Bad address 64990 go CALL writev(0x2,0x7fffffffc410,0x3) 64990 go GIO fd 2 wrote 89 bytes "cannot set up thread-local storage: cannot set %fs base address for thread-local storage " 64990 go RET writev 89/0x59 64990 go CALL linux_exit_group(0x7f) It also doesn't fix the statically linked binary, although it changes the way it fails: (gdb) run Starting program: /usr/home/en322/aosp/prebuilts/go/linux-x86/bin/gofmt Program received signal SIGSEGV, Segmentation fault. runtime.settls () at prebuilts/go/linux-x86/src/runtime/sys_linux_amd64.s:524 524 prebuilts/go/linux-x86/src/runtime/sys_linux_amd64.s: No such file or directory. (gdb) where #0 runtime.settls () at prebuilts/go/linux-x86/src/runtime/sys_linux_amd64.s:524 #1 0x0000000000453694 in runtime.rt0_go () at prebuilts/go/linux-x86/src/runtime/asm_amd64.s:145 #2 0x0000000000000000 in ?? () Now, reverting r313993 and instead applying the second chunk (set_pcb_flags()) does seem to make both dynamic and static binaries work. If instead I revert r313993 and apply the final chunk (turning linux_set_cloned_tls() into a wrapper), we're back to both binaries crashing. The binaries in question are here: https://people.freebsd.org/~trasz/go https://people.freebsd.org/~trasz/gofmt (In reply to Edward Tomasz Napierala from comment #8) Thanks. I hope to take a deeper look soon. (In reply to Conrad Meyer from comment #9) Any news on this? I got the same problem, doing as trasz wrote in comment #6 solved the issue. Thanks Hi guys, Hit this a few weeks ago haven't notice the ticket, the sysarch call will try to load the arguments from userspace and fail as they are on the kernel stack. A working (but ugly) solution is to duplicate the code in linux_machdep: --- a/sys/amd64/linux/linux_machdep.c +++ b/sys/amd64/linux/linux_machdep.c @@ -240,10 +240,14 @@ linux_arch_prctl(struct thread *td, struct linux_arch_prctl_args *args) error = EPERM; break; case LINUX_ARCH_SET_FS: - bsd_args.op = AMD64_SET_FSBASE; - bsd_args.parms = (void *)args->addr; - error = sysarch(td, &bsd_args); - if (error == EINVAL) + if (args->addr < VM_MAXUSER_ADDRESS) { + struct pcb *pcb = curthread->td_pcb; + set_pcb_flags(pcb, PCB_FULL_IRET); + pcb->pcb_fsbase = args->addr; + td->td_frame->tf_fs = _ufssel; + error = 0; + } + else error = EPERM; A better solution will be to change sysarch to accept addition parameter about the location of the memory, but haven't have the time to explore this path yet. Hope this helps. A commit references this bug: Author: dchagin Date: Sun Mar 24 14:02:57 UTC 2019 New revision: 345468 URL: https://svnweb.freebsd.org/changeset/base/345468 Log: Revert r313993. AMD64_SET_**BASE expects a pointer to a pointer, we just passing in the pointer value itself. Set PCB_FULL_IRET for doreti to restore %fs, %gs and its correspondig base. PR: 225105 Reported by: trasz@ MFC after: 1 month Changes: head/sys/amd64/linux/linux_machdep.c A commit references this bug: Author: dchagin Date: Sun Apr 28 14:30:26 UTC 2019 New revision: 346841 URL: https://svnweb.freebsd.org/changeset/base/346841 Log: MFC r345468: Revert r313993. AMD64_SET_**BASE expects a pointer to a pointer, we just passing in the pointer value itself. Set PCB_FULL_IRET for doreti to restore %fs, %gs and its correspondig base. PR: 225105 Changes: _U stable/12/ stable/12/sys/amd64/linux/linux_machdep.c Thanks to Yanko and trasz@ |