Feature Request: Add support for OpenBSD

[jesselucas]

While attempting to build wasmtime on OpenBSD 6.6 cargo build --release fails with the following error:

error[E0425]: cannot find function `get_region` in module `os`
   --> /home/j/.cargo/registry/src/github.com-1ecc6299db9ec823/region-2.1.2/src/lib.rs:133:7
    |
133 |   os::get_region(page::floor(address as usize) as *const u8)
    |       ^^^^^^^^^^ not found in `os`

[darfink]

I would gladly support it if possible. Currently there are two hurdles to overcome:

• Does OpenBSD expose any syscalls for querying memory pages? Whilst mprotect is available I can't seem to find any documentation for querying (perhaps unavailable due to its focus on safety).
• Are there any CI services that support OpenBSD? Whilst not an absolute requirement, I would very much like all supported operating system to be continuously tested.

[bjorn3]

rust-lang/rust seems to use cross compilation to test FreeBSD. It doesnt test OpenBSD, but cross compilation shouls work for that too.

[jesselucas]

Thank you @darfink ! Would https://man.openbsd.org/mquery.2 work?

[darfink]

I believe not. It seems to only provide information whether a certain memory region is free or not.

Based on the description of procmap, I would assume reading /dev/kmem is sufficient (this does require kern.allowkmem to be set).

The next step would be to configure and run OpenBSD on a virtual machine whenever I have enough spare time. From my limited searching, I can't seem to find an example output from /dev/kmem which would be a great resource for the initial implementation.

[elmarsto]

Picking this back up. I'm happy to do this implementation -- I need wasmtime to work. I've got several OpenBSD instances and I can throw up new OpenBSD 6.7 instances on Azure fairly easily.

This is near and dear and I would like to expedite :D

[darfink]

That's great to hear! I believe there are two possible solutions:

1. Implement a parser/use a C lib for /dev/kmem (also requires kern.allowkmem to be set when querying)

2. If wasmtime does not use any querying abilities, a default feature flag for querying abilities could be added (similar to what @cfallin did here cfallin/region-rs@query-feature) This would require the following functions to be disabled when not using the aforementioned feature:

   • query
   • query_range
   • protect_with_handle

   The biggest downside would be how limited the scope of the unit tests would become without any querying capability.

[bjorn3]

Cranelift only uses region::page::size and region::protect. Adding a feature flag for the querying abilities would also be nice for Redox OS.

[cfallin]

Yes, I was actually playing with this a bit a week ago, as @bjorn3 linked. I have a branch of wasmtime at cfallin/wasmtime@openbsd (which uses that hacked region-rs fork) that actually builds and runs on OpenBSD 6.8 with rustc 1.46 from pkg_add. It's not quite the cleanest, though, so there is still a bit of work in making this "proper". Working with upstream on region-rs and figuring out if a feature-based approach makes sense would be the first part, I suppose.

[cfallin]

(As an example of just-hacked-to-get-it-to-work-ness, I hardcoded the offset of RIP in the signal/trap frame for OpenBSD/x86-64; we should add that to libc or at least use a C helper that includes the appropriate system header.)

[botovq]

I looked at this a bit because this is used for preventing key material from being written to swap in a bitwarden app a friend of mine is interested in. This particular use of this crate (if it did work) seems not too great on OpenBSD, as swap is encrypted by default, so no keys will hit the disk in plaintext.

It would probably be preferable to neuter this functionality instead of adding a layer of unsafety. A feature flag along the lines sketched by @cfallin would likely be desirable, especially since OpenBSD's ABI is notoriously unstable, so exposing is as outlined below is probably risky.

If the full query functionality is desired, I do not think direct reading from /dev/kmem is the way to go. As noted, this requires kern.allowkmem to be enabled, which can only be set in single user mode, so it would require users to open up significant attack surface on their boxes.

OpenBSD supports the KERN_PROC_VMMAP sysctl which is also the kernel interface underlying FreeBSD's kinfo_getvmmap() implementation. There is no corresponding library function in OpenBSD, but from a quick look, it could probably be implemented in (unsafe) rust without too much trouble. The struct returned is defined in OpenBSD's sysctl.h:

https://github.com/openbsd/src/blob/68e47d60f0b22f2cac8b6fd3b2488a09fd279c84/sys/sys/sysctl.h#L490-L511

This is similar to, but not exactly the same as FreeBSD's struct but it should contain all information required. With that, it is probably not too hard to adapt freebsd.rs.

[darfink]

I greatly appreciate your input, this is more than enough to attempt an implementation for both OpenBSD & FreeBSD. I will pick this up again and try to have an implementation ready before the end of the month.

[darfink]

Alright, initial support for OpenBSD has been implemented in the latest master.

There are still some items that remain before a new release. Priority number one will be CI for the OpenBSD implementation (though the implementation has been tested locally).

In the long run it may still be necessary to provide a feature flag to exclude querying functionality, mostly due to OpenBSD's syscall ABI instability and it's potential move to disallow system calls from anywhere but libc.

For now, this implementation is my preference, but I will be open for re-evaluation. As an addendum, I may add that the kinfo_vmentry definition hasn't been altered since it was introduced (7 years ago).

[darfink]

I've implemented CI for OpenBSD (and FreeBSD), but there have been a lot of changes since the most recent release (2.2.0), so to avoid spending a lot of time backporting, I've decided that the next release will be a major version (3.0.0).

With that said, a lot of the API still remains compatible, so the majority of projects that depend on region will most likely be able to build with latest master as well. As for wasmtime, once updated, this library will no longer be a blocker for its OpenBSD support. To test it out today, clone the repository and add the following to its top-level Cargo.toml:

[patch.crates-io]
region = { git = 'https://github.com/darfink/region-rs', branch = 'master' }

As for the next release, it will be as soon as I'm confident with the current interface iteration, ensuring there will be no more significant changes.

[cfallin]

Hi @darfink -- thanks for this! I just updated my Wasmtime-on-OpenBSD branch using this support and it works great (see linked PR), with a few other patches to upstream to other folks as well.

Would it be possible to have a new release that includes this sometime soon?

[darfink]

Yes, I am becoming fairly confident in the current API iteration, especially since the 9461c2c41a67192259d45175f19d765504d77ff0 change. If this is a bottleneck for a wasmtime release with OpenBSD support, I could release a new version rather promptly.

[cfallin]

It looks like this is now the last non-released patch that my Wasmtime-on-OpenBSD port relies on; if you're able to release at some point, I might be able to get the Wasmtime PR merged as well. There's not really any urgency, per se, but if it's easy, it would be appreciated!

[darfink]

Version 3.0.0 has been published with support for OpenBSD