Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print error message on empty BPF #192

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/seccomp-tools/dumper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ def dump_by_pid(pid, limit, &block)
while limit.negative? || i < limit
begin
bpf = Ptrace.seccomp_get_filter(pid, i)
rescue Errno::ENOENT, Errno::EINVAL
rescue Errno::EINVAL
Logger.error('No seccomp filters installed')
break
rescue Errno::ENOENT
Logger.error('No filter exists at this index')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the test case with limit = 2, it's fine to print this message.

But for the case of limit is negative (which means no limit), we shouldn't print that message as it's expected to just stop when there is no filters, it's not an error. So let's add an if limit.positive? condition at this line.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @qua3k , do you want to make this change?

break
end
collect << (block.nil? ? bpf : yield(bpf, nil))
Expand Down
13 changes: 12 additions & 1 deletion spec/cli/dump_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,22 @@
break if line.start_with?('Welcome')
end
expect { described_class.new(['-f', 'inspect', '-p', pid.to_s]).handle }.to output(@bpf_inspect).to_stdout
expect { described_class.new(['-l', '2', '-p', pid.to_s]).handle }.to output(@bpf_disasm).to_stdout
expect { described_class.new(['-l', '2', '-p', pid.to_s]).handle }.to output(@bpf_disasm+"[ERROR] No filter exists at this index\n").to_stdout
i.write("0\n")
end
end

it 'by pid without filter' do
pid = Process.spawn('sleep 60')
begin
error = /No seccomp filters installed/
expect { described_class.new(['-p', pid.to_s]).handle }.to output(error).to_stdout
ensure
Process.kill('TERM', pid)
Process.wait(pid)
end
end

it 'by pid without root' do
pid = Process.spawn('sleep 60')
begin
Expand Down