Bug 253208 - ntpd: fails to start with PIE
Summary: ntpd: fails to start with PIE
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.0-STABLE
Hardware: Any Any
: --- Affects Some People
Assignee: Cy Schubert
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-03 11:07 UTC by Evgenii Khramtsov
Modified: 2021-12-30 15:37 UTC (History)
5 users (show)

See Also:
koobs: maintainer-feedback? (mw)
koobs: mfc-stable13?
koobs: mfc-stable12?


Attachments
Woraround ntpd fix. (355 bytes, patch)
2021-08-03 04:43 UTC, Cy Schubert
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Evgenii Khramtsov 2021-02-03 11:07:08 UTC
If one builds FreeBSD with:
WITH_BIND_NOW=yes
WITH_PIE=yes

and sets sysctls:
kern.elf64.aslr.enable=1
kern.elf64.aslr.honor_sbrk=0
kern.elf64.aslr.pie_enable=1
kern.elf64.allow_wx=0
security.bsd.stack_guard_page=1

ntpd will not start:

pid 63899 (ntpd), jid 0, uid 123: exited on signal 11

A workaround is to disable ASLR stack gap:

elfctl -e +noaslrstkgap /usr/sbin/ntpd
Comment 1 mw 2021-02-26 18:26:12 UTC
We will take a closer look at this issue.
Comment 2 Cy Schubert freebsd_committer freebsd_triage 2021-04-27 13:57:29 UTC
Can you post the ntp entries from your rc.conf, please. Also the kern.elf64.aslr and the kern.elf32.aslr sysctl MIBs.

grep ntp /etc/rc.conf
sysctl kern.elf64.aslr kern.elf32.aslr

And post the output here.
Comment 3 Evgenii Khramtsov 2021-04-27 15:32:17 UTC
(In reply to Cy Schubert from comment #2)

I no longer use it, but I restored the old config from backup:

$ grep ntp /etc/rc.conf
ntpd_enable="YES"
ntpd_sync_on_start="YES"

$ sysctl kern.elf64.aslr kern.elf32.aslr
kern.elf64.aslr.stack_gap: 3
kern.elf64.aslr.honor_sbrk: 0
kern.elf64.aslr.pie_enable: 1
kern.elf64.aslr.enable: 1
sysctl: unknown oid 'kern.elf32.aslr'

(local kernel no 32-bit)

Still reproducible on main-n246330.
Comment 4 Cy Schubert freebsd_committer freebsd_triage 2021-04-27 17:19:21 UTC
main-246330 is this commit.


commit f85ed1249773383bf80cb095b827860d80514339
Author: Andriy Gapon <avg@FreeBSD.org>
Date:   Mon Feb 4 21:50:55 2013 +0000

    ktr: copy content from the early static buffer if KTR_ENTRIES !=
    KTR_BOOT_ENTRIES
    
    Reported by:    glebius, jhb
    Pointyhat to:   avg
    MFC after:      14 days
    X-MFC with:     r246282

Notes:
    svn path=/head/; revision=246330

I think you mean something else. Can you please provide the complete uname -a output. There should be GIT hash after the n246330. The n246330 means nothing whereas the hash number will tell us the GIT commit. Uname -a please.
Comment 5 Evgenii Khramtsov 2021-04-27 17:23:24 UTC
(In reply to Cy Schubert from comment #4)

Sorry. Commit 5eb9c93a20d7320b24635ed09eba4908951bdeb2 and some local netinet6 patches.

FreeBSD vax.khramtsov.org 14.0-CURRENT FreeBSD 14.0-CURRENT #0 main-n246330-5eb9c93a20d-dirty: Mon Apr 26 20:04:38 MSK 2021     ei@vax.khramtsov.org:/usr/obj/usr/src/amd64.amd64/sys/VAX  amd64
Comment 6 Cy Schubert freebsd_committer freebsd_triage 2021-04-27 18:43:51 UTC
Ok, I can finally reproduce the problem with kern.elf64.aslr.pie_enable=1. It's not the stack gap but PIE.

Adjusting the stack to 200 from the default 50, just like OpenBSD does, resolves this issue.

The question now is, do we adjust it globally or only when PIE is enabled. I've reached out to the folks at nwtime.org for their input.
Comment 7 Cy Schubert freebsd_committer freebsd_triage 2021-04-27 20:53:43 UTC
As a workaround, until I can bake this into ntp (base and port), please put this at the bottom of your /etc/ntp.conf:

rlimit stacksize 200
Comment 8 Evgenii Khramtsov 2021-04-27 21:17:56 UTC
(In reply to Cy Schubert from comment #7)

Thanks. I can still reproduce this, even with larger values like '2000' or '20000'.
Comment 9 Evgenii Khramtsov 2021-04-27 21:24:31 UTC
(In reply to Evgeniy Khramtsov from comment #8)

Anything else to include or look for?
Comment 10 Cy Schubert freebsd_committer freebsd_triage 2021-04-27 22:53:22 UTC
So far the only way to resolve this when PIE is enabled, regardless whether the ASLR sysctl is or is not is,

elfctl -e +noaslrstkgap /usr/sbin/ntpd
Comment 11 Evgenii Khramtsov 2021-05-21 22:06:18 UTC
See https://reviews.freebsd.org/R10:af949c590bd8a00a5973b5875d7e0fa6832ea64a

Though the commit message lacks the MFC after keyword.

Leaving this PR open so stable/13 and 13 RELEASE users can find this.
Comment 12 Kubilay Kocak freebsd_committer freebsd_triage 2021-08-02 08:12:05 UTC
^Triage: Request feedback from committer of base af949c590b

@Marcin Was the workaround for setting noaslrstkgap  merged to stable/*? If not, can we look to getting it merged?

@Cy Have we heard back from upstream on permanent resolutions, or otherwise have any ideas for not requiring overriding aslr?
Comment 13 Cy Schubert freebsd_committer freebsd_triage 2021-08-03 04:43:00 UTC
Created attachment 226901 [details]
Woraround ntpd fix.

Upstream is not working on this. The only workable solution so far is MFC af949c590bd8a00a5973b5875d7e0fa6832ea64a, attached. Test it out, please, I'll MFC.
Comment 14 mw 2021-08-03 05:31:34 UTC
Hi,

We have just prepared a solution based on setrlimit, which seems to be working and more correct. I will link it here once available.
Comment 15 mw 2021-08-15 22:12:43 UTC
Please check and review the following solution, that seems more proper than disabling the stackgap for ntpd
https://reviews.freebsd.org/D31516
Comment 16 Dawid Gorecki 2021-09-10 15:33:30 UTC
Hi,

We have updated the setrlimit patch and created additional two which should resolve the problems related to stack gap. You can see all of the patches in:
https://reviews.freebsd.org/D31516
https://reviews.freebsd.org/D31897
https://reviews.freebsd.org/D31898
Comment 17 mw 2021-10-04 08:58:56 UTC
The commits got stuck on a lack of feedback.
Comment 18 Cy Schubert freebsd_committer freebsd_triage 2021-10-04 14:41:17 UTC
Commit af949c590bd8a00a5973b5875d7e0fa6832ea64a and MFC c0b03251095d38e9635d0f82e3a401d29fa4d891 work around this problem. It is suggested this woraround also be used with ports/www/firefox and ports/mail/thunderbird.

I will port this fix to ports/net/ntp.
Comment 19 mw 2021-10-04 15:09:36 UTC
Hi, 

Thanks, but the thing is we don't want to use the work around, but real fixes, as mentioned by Dawid:
https://reviews.freebsd.org/D31516
https://reviews.freebsd.org/D31897
https://reviews.freebsd.org/D31898

Best regards,
Marcin
Comment 20 Cy Schubert freebsd_committer freebsd_triage 2021-10-04 15:20:23 UTC
This is true but the workaround can be used until the reviews are approved. I've been closely monitoring the reviews. As a matter of fact, all of the NTP mitigations committed to this date will no longer be needed once the reviews are accepted.

The reviews resolve not only ntpd but also firefox and thunderbird with similar (but not the same) ASLR issues. I think this should be tracked under its own ticket given that each of these are in a loose way commonly affected by FreeBSD ASLR.
Comment 21 commit-hook freebsd_committer freebsd_triage 2021-10-15 08:24:23 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=889b56c8cd84c9a9f2d9e3b019c154d6f14d9021

commit 889b56c8cd84c9a9f2d9e3b019c154d6f14d9021
Author:     Dawid Gorecki <dgr@semihalf.com>
AuthorDate: 2021-10-13 19:01:08 +0000
Commit:     Marcin Wojtas <mw@FreeBSD.org>
CommitDate: 2021-10-15 08:21:47 +0000

    setrlimit: Take stack gap into account.

    Calling setrlimit with stack gap enabled and with low values of stack
    resource limit often caused the program to abort immediately after
    exiting the syscall. This happened due to the fact that the resource
    limit was calculated assuming that the stack started at sv_usrstack,
    while with stack gap enabled the stack is moved by a random number
    of bytes.

    Save information about stack size in struct vmspace and adjust the
    rlim_cur value. If the rlim_cur and stack gap is bigger than rlim_max,
    then the value is truncated to rlim_max.

    PR: 253208
    Reviewed by: kib
    Obtained from: Semihalf
    Sponsored by: Stormshield
    MFC after: 1 month
    Differential Revision: https://reviews.freebsd.org/D31516

 sys/kern/imgact_elf.c    |  5 +++--
 sys/kern/kern_exec.c     | 11 ++++++++---
 sys/kern/kern_resource.c |  3 +++
 sys/sys/imgact_elf.h     |  2 +-
 sys/sys/sysent.h         |  2 +-
 sys/vm/vm_map.c          |  2 ++
 sys/vm/vm_map.h          |  1 +
 7 files changed, 19 insertions(+), 7 deletions(-)
Comment 22 commit-hook freebsd_committer freebsd_triage 2021-12-30 15:37:43 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=16a900ae02427e6ab3c91282847eba553f4dcd89

commit 16a900ae02427e6ab3c91282847eba553f4dcd89
Author:     Dawid Gorecki <dgr@semihalf.com>
AuthorDate: 2021-10-13 19:01:08 +0000
Commit:     Marcin Wojtas <mw@FreeBSD.org>
CommitDate: 2021-12-30 15:24:59 +0000

    setrlimit: Take stack gap into account.

    Calling setrlimit with stack gap enabled and with low values of stack
    resource limit often caused the program to abort immediately after
    exiting the syscall. This happened due to the fact that the resource
    limit was calculated assuming that the stack started at sv_usrstack,
    while with stack gap enabled the stack is moved by a random number
    of bytes.

    Save information about stack size in struct vmspace and adjust the
    rlim_cur value. If the rlim_cur and stack gap is bigger than rlim_max,
    then the value is truncated to rlim_max.

    PR: 253208
    Reviewed by: kib
    Obtained from: Semihalf
    Sponsored by: Stormshield
    MFC after: 1 month
    Differential Revision: https://reviews.freebsd.org/D31516

    (cherry picked from commit 889b56c8cd84c9a9f2d9e3b019c154d6f14d9021)

 sys/kern/imgact_elf.c    |  5 +++--
 sys/kern/kern_exec.c     | 11 ++++++++---
 sys/kern/kern_resource.c |  3 +++
 sys/sys/imgact_elf.h     |  2 +-
 sys/sys/sysent.h         |  2 +-
 sys/vm/vm_map.c          |  2 ++
 sys/vm/vm_map.h          |  1 +
 7 files changed, 19 insertions(+), 7 deletions(-)