diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e28744..3977156 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. ## Unreleased +### Fixed + +- Remove the 100 ms child-exit polling interval from subprocess capture. Capture now drains stdout and stderr, then monitors the child via pidfd until exit or the wall-clock deadline, retaining polling only as a compatibility fallback when pidfds are unavailable. + ## [0.4] - 2026-08-10 ### Changed diff --git a/ext/landlock/landlock.c b/ext/landlock/landlock.c index 63a2026..55b8f43 100644 --- a/ext/landlock/landlock.c +++ b/ext/landlock/landlock.c @@ -121,6 +121,20 @@ static VALUE rb_ll_close_fd(VALUE self, VALUE fd_value) { return Qnil; } +static VALUE rb_ll_pidfd_open(VALUE self, VALUE pid_value) { +#ifdef SYS_pidfd_open + int fd = syscall(SYS_pidfd_open, NUM2PIDT(pid_value), 0); + if (fd < 0) { + raise_syscall_error("pidfd_open"); + } + return INT2NUM(fd); +#else + errno = ENOSYS; + raise_syscall_error("pidfd_open"); + return Qnil; +#endif +} + static VALUE rb_ll_seccomp_deny_network(VALUE self) { const char *error_message = "seccomp(SECCOMP_SET_MODE_FILTER)"; if (rb_landlock_seccomp_deny_network(&error_message) != 0) { @@ -150,6 +164,7 @@ void Init_landlock(void) { rb_define_singleton_method(mLandlock, "_add_net_rule", rb_ll_add_net_rule, 3); rb_define_singleton_method(mLandlock, "_restrict_self", rb_ll_restrict_self, 1); rb_define_singleton_method(mLandlock, "_close_fd", rb_ll_close_fd, 1); + rb_define_singleton_method(mLandlock, "_pidfd_open", rb_ll_pidfd_open, 1); rb_define_singleton_method(mLandlock, "seccomp_deny_network!", rb_ll_seccomp_deny_network, 0); rb_define_const(mLandlock, "ACCESS_FS_EXECUTE", ULL2NUM(LANDLOCK_ACCESS_FS_EXECUTE)); diff --git a/ext/landlock/landlock_native.h b/ext/landlock/landlock_native.h index d92000d..1532531 100644 --- a/ext/landlock/landlock_native.h +++ b/ext/landlock/landlock_native.h @@ -35,6 +35,10 @@ #endif #endif +#if defined(__linux__) && !defined(SYS_pidfd_open) && defined(__NR_pidfd_open) +#define SYS_pidfd_open __NR_pidfd_open +#endif + #ifndef LANDLOCK_CREATE_RULESET_VERSION #define LANDLOCK_CREATE_RULESET_VERSION (1U << 0) #endif diff --git a/lib/landlock/native.rb b/lib/landlock/native.rb index 6035f17..546c9c1 100644 --- a/lib/landlock/native.rb +++ b/lib/landlock/native.rb @@ -31,6 +31,10 @@ def close_fd(fd) Landlock.__send__(:_close_fd, fd) end + def pidfd_open(pid) + Landlock.__send__(:_pidfd_open, pid) + end + def seccomp_deny_network! Landlock.seccomp_deny_network! end diff --git a/lib/landlock/process_io.rb b/lib/landlock/process_io.rb index 904dd94..63b9d52 100644 --- a/lib/landlock/process_io.rb +++ b/lib/landlock/process_io.rb @@ -5,7 +5,7 @@ module Landlock READ_CHUNK_BYTES = 16 * 1024 - PROCESS_POLL_SECONDS = 0.1 + PID_WAIT_FALLBACK_INTERVAL_SECONDS = 0.1 STDIN_THREAD_JOIN_SECONDS = 0.1 POST_TIMEOUT_DRAIN_SECONDS = 0.05 @@ -91,45 +91,16 @@ def read_and_wait(pid, streams, timeout, max_output_bytes, truncate_output, stat timed_out = false status = nil - until streams.empty? && status + until streams.empty? if deadline remaining = deadline - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) if remaining <= 0 timed_out = true - terminate_process(pid) - status = wait_for_pid(pid) - drain_streams_until( - streams, - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) + POST_TIMEOUT_DRAIN_SECONDS, - max_output_bytes, - truncate_output, - state, - pid - ) - close_streams(streams) break end end - status ||= poll_pid(pid) - - break if streams.empty? && status - - wait = - ( - if deadline - [deadline - ::Process.clock_gettime(::Process::CLOCK_MONOTONIC), PROCESS_POLL_SECONDS].min - else - PROCESS_POLL_SECONDS - end - ) - wait = 0 if wait.negative? - if streams.empty? - sleep wait - next - end - - readable, = IO.select(streams.keys, nil, nil, wait) + readable, = IO.select(streams.keys, nil, nil, remaining) next unless readable readable.each do |io| @@ -145,16 +116,81 @@ def read_and_wait(pid, streams, timeout, max_output_bytes, truncate_output, stat end end - status ||= wait_for_pid(pid) + if deadline + status, timed_out = wait_for_pid_until(pid, deadline:) + else + status = wait_for_pid(pid) + end + + if timed_out + drain_streams_until( + streams, + ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) + POST_TIMEOUT_DRAIN_SECONDS, + max_output_bytes, + truncate_output, + state, + pid + ) + close_streams(streams) + end + [status, timed_out] end - def poll_pid(pid) - result = ::Process.wait2(pid, ::Process::WNOHANG) - result&.last + def wait_for_pid_until(pid, deadline:) + remaining = deadline - monotonic_time + if remaining <= 0 + terminate_process(pid) + return wait_for_pid(pid), true + end + + pidfd = Native.pidfd_open(pid) + pid_monitor = IO.for_fd(pidfd, autoclose: false) + readable, = IO.select([pid_monitor], nil, nil, remaining) + if !readable || monotonic_time >= deadline + terminate_process(pid) + return wait_for_pid(pid), true + end + + [wait_for_pid(pid), false] + rescue Landlock::SyscallError + wait_for_pid_until_by_polling(pid, deadline:) + ensure + close_stream(pid_monitor) if pid_monitor + Native.close_fd(pidfd) if pidfd + end + private_class_method :wait_for_pid_until + + def wait_for_pid_until_by_polling(pid, deadline:) + loop do + remaining = deadline - monotonic_time + if remaining <= 0 + terminate_process(pid) + return wait_for_pid(pid), true + end + + result = ::Process.wait2(pid, ::Process::WNOHANG) + if result + status = result.last + if monotonic_time >= deadline + terminate_process_group(pid) + return status, true + end + + return status, false + end + + IO.select(nil, nil, nil, [remaining, PID_WAIT_FALLBACK_INTERVAL_SECONDS].min) + end rescue Errno::ECHILD - nil + [nil, false] + end + private_class_method :wait_for_pid_until_by_polling + + def monotonic_time + ::Process.clock_gettime(::Process::CLOCK_MONOTONIC) end + private_class_method :monotonic_time def wait_for_pid(pid) ::Process.wait2(pid).last @@ -237,6 +273,12 @@ def terminate_process(pid) signal_process("KILL", pid) end + def terminate_process_group(pid) + ::Process.kill("KILL", -pid) + rescue Errno::ESRCH, Errno::EPERM + end + private_class_method :terminate_process_group + def signal_process(signal, pid) ::Process.kill(signal, -pid) rescue Errno::ESRCH, Errno::EPERM diff --git a/test/landlock_capture_test.rb b/test/landlock_capture_test.rb index b2d796b..a42ab29 100644 --- a/test/landlock_capture_test.rb +++ b/test/landlock_capture_test.rb @@ -105,6 +105,249 @@ def test_capture_does_not_false_timeout_after_streams_close refute result.timed_out? end + def test_capture_waits_for_child_exit_without_polling_after_streams_close + skip "Landlock unsupported" unless Landlock.supported? + + result = nil + Landlock::ProcessIO.stub(:sleep, ->(*) { flunk "capture polled for child exit" }) do + result = + Landlock.capture( + [RbConfig.ruby, "--disable=gems", "-e", "STDOUT.close; STDERR.close; sleep 0.25"], + rlimits: { + open_files: 64 + } + ) + end + + assert result.status.success? + refute result.timed_out? + end + + def test_capture_timeout_applies_after_streams_close + skip "Landlock unsupported" unless Landlock.supported? + + error = nil + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + error = + assert_raises(Landlock::CommandError) do + Landlock.capture!( + ["/bin/sh", "-c", "exec 1>&- 2>&-; exec /bin/sleep 30"], + rlimits: { + open_files: 64 + }, + timeout: 0.1 + ) + end + end + + assert error.result.timed_out? + refute_nil error.status + assert error.status.signaled? + end + + def test_capture_rechecks_deadline_after_pidfd_becomes_readable + skip "Landlock unsupported" unless Landlock.supported? + + pid_monitors = [] + original_for_fd = IO.method(:for_fd) + original_select = IO.method(:select) + for_fd = ->(*arguments, **options) { original_for_fd.call(*arguments, **options).tap { |io| pid_monitors << io } } + select = + lambda do |readers, writers = nil, errors = nil, timeout = nil| + if readers&.any? { |io| pid_monitors.include?(io) } + original_select.call(readers, writers, errors, 5) + else + original_select.call(readers, writers, errors, timeout) + end + end + + result = nil + IO.stub(:for_fd, for_fd) do + IO.stub(:select, select) do + result = + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.15"], rlimits: { open_files: 64 }, timeout: 0.1) + end + end + + assert_predicate result, :timed_out? + assert_predicate result.status, :success? + end + + def test_capture_fallback_kills_descendants_without_signaling_reaped_pid_when_deadline_expires_during_poll + skip "Landlock unsupported" unless Landlock.supported? + + Dir.mktmpdir do |dir| + pidfile = File.join(dir, "descendant.pid") + pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::ENOSYS::Errno) + original_wait2 = Process.method(:wait2) + original_kill = Process.method(:kill) + child_reaped = false + wait2 = + lambda do |*arguments| + sleep 0.15 if arguments.last == Process::WNOHANG + original_wait2.call(*arguments).tap { |result| child_reaped = true if result } + end + kill = + lambda do |signal, target| + flunk "capture signaled a reused PID after reaping the child" if child_reaped && target.positive? + + original_kill.call(signal, target) + end + + result = nil + Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do + Process.stub(:wait2, wait2) do + Process.stub(:kill, kill) do + result = + Landlock.capture( + [ + RbConfig.ruby, + "--disable=gems", + "-e", + "pid = Process.fork { STDOUT.close; STDERR.close; sleep 30 }; File.write(ARGV.fetch(0), pid); STDOUT.close; STDERR.close", + pidfile + ], + read: runtime_paths, + write: [dir], + execute: runtime_paths, + env: { + "PATH" => ENV.fetch("PATH", "") + }, + unsetenv_others: true, + timeout: 0.1 + ) + end + end + end + + assert_predicate result, :timed_out? + assert_predicate result.status, :success? + assert_path_exists pidfile + refute_process_alive Integer(File.read(pidfile)) + ensure + kill_process_from_file(pidfile) + end + end + + def test_capture_does_not_create_timeout_thread_after_streams_close + skip "Landlock unsupported" unless Landlock.supported? + + result = nil + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + result = + Landlock.capture( + [RbConfig.ruby, "--disable=gems", "-e", "STDOUT.close; STDERR.close; sleep 0.1"], + rlimits: { + open_files: 64 + }, + timeout: 5 + ) + end + + assert result.status.success? + refute result.timed_out? + end + + def test_capture_closes_pid_monitor_when_waiting_for_child_raises + skip "Landlock unsupported" unless Landlock.supported? + + pid_monitors = [] + wait_calls = 0 + original_for_fd = IO.method(:for_fd) + original_wait_for_pid = Landlock::ProcessIO.method(:wait_for_pid) + for_fd = ->(*arguments, **options) { original_for_fd.call(*arguments, **options).tap { |io| pid_monitors << io } } + wait_for_pid = + lambda do |pid| + wait_calls += 1 + raise IOError, "wait failed" if wait_calls == 1 + + original_wait_for_pid.call(pid) + end + + IO.stub(:for_fd, for_fd) do + Landlock::ProcessIO.stub(:wait_for_pid, wait_for_pid) do + assert_raises(IOError) do + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 0.1"], rlimits: { open_files: 64 }, timeout: 10) + end + end + end + + assert_equal 1, pid_monitors.size + assert_predicate pid_monitors.first, :closed? + end + + def test_capture_closes_raw_pidfd_when_wrapping_it_raises + skip "Landlock unsupported" unless Landlock.supported? + + pidfd = nil + closed_pidfds = [] + original_pidfd_open = Landlock::Native.method(:pidfd_open) + original_close_fd = Landlock::Native.method(:close_fd) + pidfd_open = ->(pid) { original_pidfd_open.call(pid).tap { |fd| pidfd = fd } } + close_fd = + lambda do |fd| + closed_pidfds << fd if fd == pidfd + original_close_fd.call(fd) + end + + Landlock::Native.stub(:pidfd_open, pidfd_open) do + Landlock::Native.stub(:close_fd, close_fd) do + IO.stub(:for_fd, ->(*) { raise IOError, "wrap failed" }) do + assert_raises(IOError) do + Landlock.capture(["/bin/sh", "-c", "exec 1>&- 2>&-; sleep 30"], rlimits: { open_files: 64 }, timeout: 10) + end + end + end + end + + refute_nil pidfd + assert_equal [pidfd], closed_pidfds + end + + def test_capture_falls_back_without_a_timeout_thread_when_pidfd_is_unavailable + skip "Landlock unsupported" unless Landlock.supported? + + pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::EPERM::Errno) + result = nil + Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + result = + Landlock.capture( + [RbConfig.ruby, "--disable=gems", "-e", "STDOUT.close; STDERR.close; sleep 0.1"], + rlimits: { + open_files: 64 + }, + timeout: 5 + ) + end + end + + assert result.status.success? + refute result.timed_out? + end + + def test_capture_fallback_enforces_timeout_when_pidfd_is_unavailable + skip "Landlock unsupported" unless Landlock.supported? + + pidfd_error = Landlock::SyscallError.new("pidfd_open", Errno::ENOSYS::Errno) + result = nil + Landlock::Native.stub(:pidfd_open, ->(*) { raise pidfd_error }) do + Thread.stub(:new, ->(*) { flunk "capture created a timeout thread" }) do + result = + Landlock.capture( + ["/bin/sh", "-c", "exec 1>&- 2>&-; exec /bin/sleep 30"], + rlimits: { + open_files: 64 + }, + timeout: 0.1 + ) + end + end + + assert result.timed_out? + assert_predicate result.status, :signaled? + end + def test_capture_does_not_wait_forever_for_blocked_stdin_reader skip "Landlock unsupported" unless Landlock.supported?