Skip to content

Draft: seccomp syscall and ioctls #1458

New issue

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

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

Already on GitHub? # to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rusty-snake
Copy link
Contributor

This is an early draft seeking for feedback on the API. The SecureComputingFilter is based on my own seccomp abstractions https://codeberg.org/crabjail/crablock/src/commit/c6cadc33b0e605bf16b8dc2fc0ba8156c7693567/seccomp/src/bpf.rs#L416 .

Closes #1451

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

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

At a first glance, this approach looks good.

operation: super::types::SeccompOperation,
flags: Option<SetSecureComputingFilterFlags>,
args: *mut c::c_void,
) -> io::Result<c::c_int> {
Copy link
Member

Choose a reason for hiding this comment

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

This function should be unsafe because it effectively dereferences args.

operation: SeccompOperation,
flags: Option<SetSecureComputingModeFilterFlags>,
args: *mut c::c_void,
) -> io::Result<()> {
Copy link
Member

Choose a reason for hiding this comment

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

This function should also be unsafe.

Self(
c::sock_fprog {
// usize as u16 is lossy. However filter programs with more than BPF_MAXINSNS (4096)
// will be rejected by the kernel with EINVAL.
Copy link
Member

Choose a reason for hiding this comment

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

If the user passes a filter list that's too long, it seems like it's be better to fail than to silently truncate the list. The user may be assuming that if the call succeeds, the entire filter list is installed.

Copy link
Contributor Author

@rusty-snake rusty-snake May 1, 2025

Choose a reason for hiding this comment

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

The kernel will return EINVAL for len > 4096 (~32kB filter code). u16::MAX is 65535 (~500kb filter code). So this is more a theoretical than a practical issue - the filter will get rejected by the kernel. On the other hand u16::try_from(length).map_err(|_| EINVAL)? isn't complicated. edit: This would mean we can not use From. So under the assumption to keep using From we can:

  • u16::try_from(length).unwrap_or(u16::MAX)
  • u16::try_from(length).unwrap_or(0) (Kernel: if (fprog->len == 0 || fprog->len > BPF_MAXINSNS) return ERR_PTR(-EINVAL);)
  • assert!(filter_lines.len() < u16::MAX as usize) as it should not happen in practice.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set_secure_computing_mode(SecureComputingMode::Filter) has no filter argument
2 participants