In D23659#520598, @jeff wrote:

So this looks to me like we made a bad topology and then worked around it by weakening the search.

Seems fine to me.

WIP, I'm in progress of drafting a private email response as well.

My questions on the initial review were never really answered...

Generally please upload phabricator diffs with more context. The arcanist tool can help do that easily. Or if you prefer the manual process, please generate patches with -U99999.

In D23223#515944, @rlibby wrote:

This sounds reasonable to me, I just wasn't personally confident enough that it was true. Is there a maintainer who can speak to this? There seems to be a difference, e.g. auditd_reap_children(), and whatever is going on with launchd (but doesn't apply to us?).

I don't see any reason auditd cannot just use the default stop action, which sends SIGTERM. auditd handles sigterm and performs more or less the same shutdown actions as those triggered by audit -t (AUDIT_TRIGGER_CLOSE_AND_DIE). I would suggest just removing the auditd_stop() function entirely.

Seems like there is a possible race condition here where $rc_pid exits and is recycled before we check for exit.

I don't object to /root mode 750. This seems to line up with, e.g., Fedora Linux defaults.

Hm, I don't understand where we unlock in the rest of the chunks without the changes. Maybe the breaks intentionally return a locked context, but the continues seem obviously wrong. I don't see an unlock in, e.g., libusb_hotplug_filter() which would make that safe.

Updated patch LGTM :-). I see philip has already committed it, great!

LGTM other than the indentation

LGTM. 8kB is overkill for the key, but mostly harmless.

Thanks! LGTM other than the remaining EINVAL <-> ACPI_STATUS conflict in the revision.

Oh whoops I forgot about the error handling issue.

Thanks, the updates look great. I'm still curious about the disassembly of the methods if you can get it, but I don't think that's a blocker issue.

LGTM modulo whitespace nit and ACPI_STATUS return to userspace in set.

Add sysctl knob to disable VDSO page ABI.

Fix pasto in ifndef conditional

In D23289#510876, @kevans wrote:

I had some concerns about providing *_r variants of these that match names from glibc but aren't compatible, but I see the ultimate plan is that these aren't intended to be exported from libc at all- this seems OK.

Thanks for the quick reviews, Mark!

Thanks, looks good to me except for one concern below.