Move all seccomp consts and structs into top-level mod

[boustrophedon]

Move all seccomp consts and structs into top-level mod

Seccomp constants and structs were partially defined in the top-level mod.rs for linux and partially outside. This commit moves everything into the top-level mod and adds missing entries as of linux 6.4.12.

Resolves #3342 and rust-vmm/seccompiler#57

I couldn't get main to run libc-test cargo test (even without my changes) with either gcc 13.2.1 or clang 16.0.6. They failed with different errors/warnings.

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[boustrophedon]

style check is failing but I get

$ rustc ci/style.rs && ./style src
good style!

It looks like it was some indentation issue. Either way, I fixed it.

[boustrophedon]

The linux failures are coming from SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV which was added last year torvalds/linux@c2aa2df

Looks like it was available in about 5.19 or so, and the docker image this is running in is ubuntu-22.04 which is running 5.15 according to this page.

I'm not sure what to do here.

[boustrophedon]

Hi, anyone have any suggestions for what to do here?

The issue is that the version of the Linux kernel in CI does not support some newer constants for the seccomp syscall, and so CI fails when checking whether the constants exist in the corresponding C headers.

However, the version in CI is also newer than the minimum supported version of the linux kernel so in theory this crate may already have constants that are not supported by the minimum version but do happen to be supported by the version in CI.

[JohnTitor]

Could you try to do a rebase? We recently updated the Ubuntu image on CI to 23.10, which uses Linux v6.5.

[boustrophedon]

@JohnTitor Thanks, it looks like everything is passing now!

[JohnTitor]

Cool! One concern is that our musl toolchain is quite old and might fail, but let's see if that's the case.
If fails, you can ignore tests like:

libc/libc-test/build.rs

Lines 3503 to 3506 in 240de1f

	// FIXME: requires >= 5.4.1 kernel headers
	"pgn_t" if musl => true,
	"priority_t" if musl => true,
	"name_t" if musl => true,

@bors r+

[bors]

📌 Commit 6621f07 has been approved by JohnTitor

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6621f07 with merge 13ffd24...

[bors]

💔 Test failed - checks-actions

[boustrophedon]

Yeah, it looks like it's failing on musl with stuff like

cargo:warning=/checkout/target/aarch64-unknown-linux-musl/debug/build/libc-test-dd9ce03b4e008979/out/main.c: In function '__test_field_type_seccomp_notif_sizes_seccomp_data':

  cargo:warning=/checkout/target/aarch64-unknown-linux-musl/debug/build/libc-test-dd9ce03b4e008979/out/main.c:21349:30: error: invalid use of undefined type 'struct seccomp_notif_sizes'

  cargo:warning=/checkout/target/aarch64-unknown-linux-musl/debug/build/libc-test-dd9ce03b4e008979/out/main.c:21700:38: error: invalid use of undefined type 'struct seccomp_notif_addfd'

Could I help out with upgrading musl? If you think it's too much work I'm fine with just adding the exceptions for the failing types and constants.

[JohnTitor]

Could I help out with upgrading musl? If you think it's too much work I'm fine with just adding the exceptions for the failing types and constants.

Sadly it's quite hard. We have to upgrade musl from v1.1.24 to v1.2 or higher, but it would break the compatibility. See #3068 for ongoing work. I'd recommend just skipping tests for now.

[boustrophedon]

I'm not sure if I'm able to trigger a bors run to test the tier2 targets.

[JohnTitor]

tier2 targets are only run on bors (otherwise it consumes a lot of GHA budget on the org), @bors r+

[bors]

📌 Commit 9c92649 has been approved by JohnTitor

It is now in the queue for this repository.

[bors]

💔 Test failed - checks-actions

[boustrophedon]

So I'm trying to reproduce the failure locally and the first issue I ran into was

+ cargo test --no-default-features --manifest-path libc-test/Cargo.toml --target mips-unknown-linux-gnu
    Updating crates.io index
error: failed to write /checkout/Cargo.lock

Caused by:
  failed to open: /checkout/Cargo.lock  

Caused by:
  Read-only file system (os error 30)

but the docker run command is https://github.com/rust-lang/libc/blob/main/ci/run-docker.sh#L48
which contains

      --volume "$(rustc --print sysroot)":/rust:ro \
      --volume "$(pwd)":/checkout:ro \

so I changed them to rw and execution could continue. Now I'm getting

+ cargo test --no-default-features --manifest-path libc-test/Cargo.toml --target mips-unknown-linux-gnu
    Updating crates.io index
 Downloading crates ...
  Downloaded rustc_version v0.4.0
error: failed to download replaced source registry `crates-io`

Caused by:
  failed to create directory `/cargo/registry/cache/index.crates.io-6f17d22bba15001f`

Caused by:
  Permission denied (os error 13)

and I don't immediately see how to fix this. I'm not sure if the docker user is supposed to be root or something or if something else is wrong with the docker bindmounts.

[boustrophedon]

Just for reference, the failure in CI looks like

PASSED 20 tests
     Running test/linux_strerror_r.rs (target/mips-unknown-linux-gnu/debug/deps/linux_strerror_r-cbe4f10440c1c52a)
RUNNING ALL TESTS
PASSED 1 tests
     Running test/linux_termios.rs (target/mips-unknown-linux-gnu/debug/deps/linux_termios-16515f471baa64e5)
RUNNING ALL TESTS
PASSED 50 tests
     Running test/main.rs (target/mips-unknown-linux-gnu/debug/deps/main-bf0b31ae08956b48)
RUNNING ALL TESTS
rust[4] = 0 != 4 (C): Rust "seccomp_notif_sizes" -> C
rust[5] = 0 != 5 (C): Rust "seccomp_notif_sizes" -> C
thread 'main' panicked at 'some tests failed', /checkout/target/mips-unknown-linux-gnu/debug/build/libc-test-b013fc4656104f6c/out/main.rs:12:21

Earlier in the docker run, it does say

#6 7.161 Get:111 http://archive.ubuntu.com/ubuntu mantic-updates/main amd64 linux-headers-6.5.0-10 all 6.5.0-10.10 [13.4 MB]

so it's not like there are old headers or anything. Here's the definition of seccomp_notif_sizes: https://github.com/torvalds/linux/blob/master/include/uapi/linux/seccomp.h#L69

[boustrophedon]

Aha, I took another look at the docker command:

    docker run \
      --rm \
      --user "$(id -u)":"$(id -g)" \

this is probably expecting that the docker command is run by root so that $(id -u) and -g evaluate to 0. I don't really think this makes sense since ideally your local user and docker user have no relationship to each other.

So after I changed them to be 0 for root, I'm getting simply that it appears the mips target isn't installed.

error[E0463]: can't find crate for `core`
  |
  = note: the `mips-unknown-linux-gnu` target may not be installed
  = help: consider downloading the target with `rustup target add mips-unknown-linux-gnu`

so I'll do some more digging. Maybe I just need to delete the image and restart or something

(edit: deleted some stuff about the --rm flag.)

[JohnTitor]

So, we have to build the target using -Zbuild-std now, as rust-lang/compiler-team#648 dropped the support. As it's now a tier 3 target, we could downgrade our CI support for MIPS instead. Let me do that, one sec.

[boustrophedon]

@JohnTitor Does docker CI run locally for you? It might be an issue with my environment just not matching what the scripts want.

[Arnavion]

Does docker CI run locally for you?

Yes it works fine.

printf '[toolchain]\nchannel = "nightly"\n' >rust-toolchain.toml
rustup target add mips-unknown-linux-gnu
cargo generate-lockfile
./ci/run-docker.sh mips-unknown-linux-gnu

(Replace mips-unknown-linux-gnu with whatever target you want to test.)

+ cargo test --no-default-features --manifest-path libc-test/Cargo.toml --target mips-unknown-linux-gnu
    Updating crates.io index
error: failed to write /checkout/Cargo.lock

Caused by:
  failed to open: /checkout/Cargo.lock  

Caused by:
  Read-only file system (os error 30)

This is because you didn't run cargo generate-lockfile

I don't really think this makes sense since ideally your local user and docker user have no relationship to each other.

No, it expects the user running the script to be in the docker group, which is a standard thing to do.

So after I changed them to be 0 for root, I'm getting simply that it appears the mips target isn't installed.

This is because you didn't run rustup target add

[Arnavion]

Anyway I rebased your branch on main and test all the targets except mips and android for you and they all pass. So if you rebase and ask JohnTitor to rerun CI it should be fine.

[boustrophedon]

I rebased, thanks for helping out with this! @JohnTitor could you re-try CI please?

It might be nice to add instructions in the ci/ directory on how to run everything. I also am not sure if the solution to the uid and bindmount ro/rw stuff is to just run the scripts as root locally, which seems not ideal, or if there's a better way besides modifying the scripts.

[Arnavion]

I also am not sure if the solution to the uid and bindmount ro/rw stuff is to just run the scripts as root locally, which seems not ideal, or if there's a better way besides modifying the scripts.

As I said, add yourself to the docker group.

[boustrophedon]

Sorry, I forgot to mention my user is in the docker group!

[boustrophedon]

Hi again, sorry to bother, @JohnTitor could you re-run the CI tests when you have a chance? Thank you!

[JohnTitor]

Sorry for the long delay! I missed this. @bors r+

[bors]

📌 Commit 466516d has been approved by JohnTitor

It is now in the queue for this repository.

[bors]

⌛ Testing commit 466516d with merge d3238e0...

[bors]

☀️ Test successful - checks-actions, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing d3238e0 to main...

[boustrophedon]

Thank you!