`seccomp_export_bpf_mem` about libseccomp-rs HOT 12 CLOSED

To handle the case (dynamically linking), we have supported for the error handling with good error message at runtime as well https://github.com/libseccomp-rs/libseccomp-rs/blob/main/libseccomp/src/notify.rs#L163C9-L163C25

It case of missing symbols, ld-linux.so will fail and the application will not start.

from libseccomp-rs.

2.2. SCMP_VER with cc crate

• cons
  • build dependency on gcc/clang

I prefer to keep using pkgconfig (with all it's downsides).

from libseccomp-rs.

@ManaSugi I've a working draft for the actual feature itself.

However, seccomp_export_bpf_mem and seccomp_precompute are new symbols. And using strong symbols for them like for all the other functions means libseccomp 2.6.0 is required to build and run if the are linked (=used by any code path). Weak symbols on the other hand are a very hacky thing in Rust (but possible and some FFI crates make heavy usage of them).

TL;DR: The question is where do we want to fail? At link time (compiler for static builds or compiler + dynamic linker for non-static builds) or at runtime.

from libseccomp-rs.

@rusty-snake

I have tried to avoid the failure at link time of the libseccomp using pkgconfig and #![cfg(libseccomp_v2_5)].
As you know, our codes of the notify feature that requires >=2.5.0 are actually built only when the libseccomp is >=2.50.
This can prevent safely users from using the feature on <2.5.0 and print a good error message.

Example:

error[E0599]: no method named `get_notify_fd` found for struct `ScmpFilterContext` in the current scope
   --> *************
    |
205 |             ctx.get_notify_fd()
    |                 ^^^^^^^^^^^^^ method not found in `ScmpFilterContext`

For more information about this error, try `rustc --explain E0599`.

Without this feature, the error message will be as follows.
This is a raw and huge error message.

error: linking with `cc` failed: exit status: 1
*********************
a lot of information
*********************
..............
          ***notify.rs:89: undefined reference to `seccomp_notify_fd'
          collect2: error: ld returned 1 exit status

  = note: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
  = note: use the `-l` flag to specify native libraries to link
  = note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)

In addition, pkgconfig and #![cfg(libseccomp_v2_5)] help our test run properly on <2.5.0.
On <2.5.0, the test codes related to the notify are not built and the other tests run properly without any problem.
Without it, the test will fail at link time due to undefined reference like the above message.

On the other hand, I really have wanted to remove the dependency of pkgconfig in order to improve maintainability.
I found the pkgconfig brought about a problem while I was preparing for the integration test of the libseccomp on ManaSugi/libseccomp@e604063 . To build our libseccomp-rs on the CI environment of the libseccomp, I have to create a pkgconfig file in advance and it is not beautiful.

Anyway, I like to make the libseccomp-rs fail at link time safely (early detection) because the weak symbol can bring about a failure at runtime (late detection). So, it's good time to reconsider the solution without pkgconfig.
About our unit tests, we have to make the new functions not compiled with some features or other.

from libseccomp-rs.

Ideas to query the version w/ pkgconfig:

• cc crate and compile + run a small binary that prints the version.
• link or ldopen libseccomp in build.rs and call seccomp_version.

Anyway I will open a PR with a cfg(libseccomp_v2_6) and then let's see.

from libseccomp-rs.

@ManaSugi what do you think about doing the following in build.rs?

//! build-dependency: libseccomp-sys

let Some(version) = unsafe { seccomp_version().as_ref() } else {
    panic!("Fatal error that actually should never happen");
};

if version.major >= 2 {
    if version.minor >= 5 {
        println!("cargo:rustc-cfg=libseccomp_v2_5");
    }
    if version.minor >= 6 {
        println!("cargo:rustc-cfg=libseccomp_v2_6");
    }
}

from libseccomp-rs.

@rusty-snake Looks good! Could you add the change to #217 and check whether the change can pass CI or not?

from libseccomp-rs.

Thinking of cross compilation. The build script is executed on the host and compiled with the host toolchain.

To come also back to the compile time check. If you compile a binary on a system with libseccomp 2.6.0, copy it to a system/container with libseccomp 2.5.x, it will fail at runtime.

from libseccomp-rs.

Maybe we can do it similar to the openssl crate https://github.com/sfackler/rust-openssl/blob/50787ed35bf9efa9dd3cbb1993a2564014b67489/openssl-sys/build/main.rs#L142. It uses cc to expand macros. SCMP_VER_* will fit for us.

from libseccomp-rs.

To come also back to the compile time check. If you compile a binary on a system with libseccomp 2.6.0, copy it to a system/container with libseccomp 2.5.x, it will fail at runtime.

Yes, but such users should link the libseccomp statically (or carefully deal with the case under the user's responsibility). To handle the case (dynamically linking), we have supported for the error handling with good error message at runtime as well https://github.com/libseccomp-rs/libseccomp-rs/blob/main/libseccomp/src/notify.rs#L163C9-L163C25

Maybe we can do it similar to the openssl crate https://github.com/sfackler/rust-openssl/blob/50787ed35bf9efa9dd3cbb1993a2564014b67489/openssl-sys/build/main.rs#L142. It uses cc to expand macros. SCMP_VER_* will fit for us.

Thanks for the proposal! Using SCMP_VER_* looks good. But, this would be large amount of codes compared to using pkgconfig.

from libseccomp-rs.

It case of missing symbols, ld-linux.so will fail and the application will not start.

Ah, yes, you're right. Users have to take care of the case under their responsibility.

Here is our options

1. Continue to use pkgconfig

   • pros
     • Make the code simple in build.rs
   • cons
     • build fail without pkgconfig (don't worry about it too much because of libseccomp has pkgconfig)

2. Use seccomp_version() or SCMP_VER with cc crate

   • pros
     • No dependency to pkgconfig
   • cons
     • Make the code in build.rs complicated and huge.

from libseccomp-rs.

Okay, let's continue to use pkgconfig. Thank you for your various proposal! (we could conclude pkgconfig is the best choice as of now)

from libseccomp-rs.