From d27a4559aace0294c48734ef58a9fffc14c05fd1 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 1 May 2025 12:28:25 +0200 Subject: [PATCH 1/3] Revert "Update util.rb to fix closing of file." This is broken and results in not closing any file descriptors. --- lib/puppet/util.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index 66be0f7da3..06c7b11815 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -479,7 +479,7 @@ def safe_posix_fork(stdin = $stdin, stdout = $stdout, stderr = $stderr, &block) begin Dir.foreach('/proc/self/fd') do |f| - if %{^\d+$}.match?(f) && f.to_i >= 3 + if f != '.' && f != '..' && f.to_i >= 3 begin IO.new(f.to_i).close rescue From 8a6bf3e8ae25f7dd70c4d4eab6e1c6d6e08a8d76 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 5 Mar 2025 14:01:47 +0100 Subject: [PATCH 2/3] Avoid closing directory we're iterating Ruby 3.4 started error checking directory access and starts to raise Errno::EBADF. This particular loop iterates on all open file descriptors and one is the directory listing from Dir.foreach. In the past this could have led to leaked file descriptors, but it's unlikely since it's likely the last opened file descriptor and have the highest number. Link: https://github.com/ruby/ruby/commit/f2919bd11c570fc5f5440d1f101be38f61e3d16b Link: https://bugzilla.redhat.com/show_bug.cgi?id=2349352 --- lib/puppet/util.rb | 6 ++++-- spec/unit/util_spec.rb | 12 +++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index 06c7b11815..37b37d614d 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -478,8 +478,10 @@ def safe_posix_fork(stdin = $stdin, stdout = $stdout, stderr = $stderr, &block) $stderr = STDERR begin - Dir.foreach('/proc/self/fd') do |f| - if f != '.' && f != '..' && f.to_i >= 3 + d = Dir.new('/proc/self/fd') + ignore_fds = ['.', '..', d.fileno.to_s] + d.each_child do |f| + if !ignore_fds.include?(f) && f.to_i >= 3 begin IO.new(f.to_i).close rescue diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb index 192f13bfb9..a819c71459 100644 --- a/spec/unit/util_spec.rb +++ b/spec/unit/util_spec.rb @@ -616,18 +616,20 @@ def withenv_utf8(&block) fds.each do |fd| if fd == '.' || fd == '..' next - elsif ['0', '1', '2'].include? fd + elsif ['0', '1', '2', '5'].include? fd expect(IO).not_to receive(:new).with(fd.to_i) else expect(IO).to receive(:new).with(fd.to_i).and_return(double('io', close: nil)) end end - dir_expectation = receive(:foreach).with('/proc/self/fd') + dir = double(Dir, fileno: '5') + dir_expectation = receive(:each_child) fds.each do |fd| dir_expectation = dir_expectation.and_yield(fd) end - allow(Dir).to dir_expectation + allow(dir).to dir_expectation + allow(Dir).to receive(:new).with('/proc/self/fd').and_return(dir) Puppet::Util.safe_posix_fork end @@ -636,7 +638,7 @@ def withenv_utf8(&block) # letting it actually close fds, which seems risky (0..2).each {|n| expect(IO).not_to receive(:new).with(n)} (3..256).each {|n| expect(IO).to receive(:new).with(n).and_return(double('io', close: nil)) } - allow(Dir).to receive(:foreach).with('/proc/self/fd').and_raise(Errno::ENOENT) + allow(Dir).to receive(:new).with('/proc/self/fd').and_raise(Errno::ENOENT) Puppet::Util.safe_posix_fork end @@ -646,7 +648,7 @@ def withenv_utf8(&block) # letting it actually close fds, which seems risky (0..2).each {|n| expect(IO).not_to receive(:new).with(n)} (3..256).each {|n| expect(IO).to receive(:new).with(n).and_return(double('io', close: nil)) } - allow(Dir).to receive(:foreach).with('/proc/self/fd').and_raise(Errno::ENOTDIR) + allow(Dir).to receive(:new).with('/proc/self/fd').and_raise(Errno::ENOTDIR) Puppet::Util.safe_posix_fork end From 3f344ab7eeff90174d1edccfe212fbaff7b403b8 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Wed, 5 Mar 2025 14:29:07 +0100 Subject: [PATCH 3/3] Please RuboCop --- lib/puppet/util.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index 37b37d614d..9270bce3e2 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -481,12 +481,12 @@ def safe_posix_fork(stdin = $stdin, stdout = $stdout, stderr = $stderr, &block) d = Dir.new('/proc/self/fd') ignore_fds = ['.', '..', d.fileno.to_s] d.each_child do |f| - if !ignore_fds.include?(f) && f.to_i >= 3 - begin - IO.new(f.to_i).close - rescue - nil - end + next if ignore_fds.include?(f) || f.to_i < 3 + + begin + IO.new(f.to_i).close + rescue + nil end end rescue Errno::ENOENT, Errno::ENOTDIR # /proc/self/fd not found, /proc/self not a dir