Disable stack gap for ntpd during build.
ClosedPublic
Actions

Authored by dgr_semihalf.com on Apr 2 2021, 2:09 PM.

Details

Reviewers

mw
emaste
kib
imp
cy

Commits

rGc0b03251095d: Disable stack gap for ntpd during build.
rGaf949c590bd8: Disable stack gap for ntpd during build.

Summary

When starting, ntpd calls setrlimit(2) to limit maximum size of its
stack. The stack limit chosen by ntpd is 200K, so when stack gap
is enabled, the stack gap is larger than this limit, which results
in ntpd crashing.

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

dgr_semihalf.com created this revision.Apr 2 2021, 2:09 PM

Herald added a subscriber: cy. · View Herald TranscriptApr 2 2021, 2:09 PM

D29551 and D29552 are required for this to work.

dgr_semihalf.com mentioned this in D27666: Enable ASLR by default for 64-bit executables..Apr 2 2021, 2:14 PM

Would it be better to tweak the stack size? ntpd does this as one of many measure to stay swapped in...

Yes, this could be resolved by changing the default stack resource limit from 200KiB to around 16MiB (maximum stack gap that can be created with default settings is 15.36MiB - 3% of default 512MiB stack resource limit). In that case the stack gap would actually exist, so any benefits resulting from it would still be there. Of course, the disadvantage of this approach is the increased resource utilization but that is expected with ASLR anyway. Also, if anyone changes the default stack limit to a lower value, this could result in segfault for them, so I think that information about that should be included somewhere in documentation.

I didn't want to touch contrib but if there is no problem with that, I can close this revision and prepare the patch for ntpd itself.

D29832 changes the default stack size for ntpd, so I'm abandoning this revision. What do you think about D29550, D29551 and D29552?

dgr_semihalf.com mentioned this in D29832: ntpd: Change default stack limit to 4096 pages..Apr 26 2021, 9:37 AM

Re-opening this. As mentioned in D29832, @kib prefers this version.

kib accepted this revision.Apr 26 2021, 12:13 PM

This revision is now accepted and ready to land.Apr 26 2021, 12:13 PM

This is already done at line 447 of ntpd.c. Why do we need this?

This revision now requires changes to proceed.Apr 26 2021, 5:42 PM

kd added a subscriber: kd.Apr 27 2021, 9:53 AM

This is probably the only solution, as it works with PIE enabled, regardless of whether ASLR is enabled or not. See discussion at D29832.

This revision is now accepted and ready to land.Apr 27 2021, 10:25 PM

I will do a little more testing and commit.

In D29553#673797, @cy wrote:

I will do a little more testing and commit.

OK, just a reminder that without D29550, D29551 and D29552 this will not work. Currently there is no infrastructure in FreeBSD build system to use elfctl during build and those patches add that.

In D29553#673827, @dgr_semihalf.com wrote:

In D29553#673797, @cy wrote:

I will do a little more testing and commit.

OK, just a reminder that without D29550, D29551 and D29552 this will not work. Currently there is no infrastructure in FreeBSD build system to use elfctl during build and those patches add that.