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

Can this replace/deprecate mach? #13

Closed
thomcc opened this issue Sep 7, 2022 · 5 comments
Closed

Can this replace/deprecate mach? #13

thomcc opened this issue Sep 7, 2022 · 5 comments

Comments

@thomcc
Copy link

thomcc commented Sep 7, 2022

Oh! I had no idea this existed, that's great. It's probably worth asking @fitzgen if they'd be willing to deprecate mach existing one in favor of this (or transfer ownership).

Last week I noticed how bad mach was, and took a stab at cleaning it up and updating it (I figured I'd have to publish it as a new version, so I didn't keep changes minimal and used new core::ffi stuff, etc), but I never quite got around to finishing. Had I known about this crate, I wouldn't have bothered with any of that!

I think nobody should be using that one, a low level crate like that can't really go unmaintained, and I don't know that everything it has is fully right (several constants changed, and structures seemed wrong for aarch64 macos). So we should see if we can make this more widely known, at the very least.

@JohnTitor
Copy link
Owner

It's probably worth asking @fitzgen if they'd be willing to deprecate mach existing one in favor of this (or transfer ownership).

I asked @fitzgen on fitzgen/mach#63 (comment) and Twitter but they just ignored me :( I guess they aren't interested in the mach crate and its maintenance at all...

If you have a good idea, let me know! I'm happy to help things as long as I can :)

@thomcc
Copy link
Author

thomcc commented Sep 7, 2022

We could submit a rustsec unmaintained notice: https://github.com/rustsec/advisory-db/blob/main/HOWTO_UNMAINTAINED.md. If it has any badly incorrect definitions (like wrong function sigs or struct layouts), a stronger advisory would be warranted.

Concerning, a lot of code still uses mach (>5M downloads compared to mach2 which has under 50k... less than 1/100th the amount).


So, it's worth considering if any of the issues it has are worth an advisory. Looking at my diff (which is mostly irrelevant), the most changes I made that were most concerning are:

  1. The mach_vm_read_list function taking a Rust array (mach_vm_read_entry_t is a typedef defined as [mach_vm_read_entry; VM_MAP_ENTRY_MAX as usize]) by value. This is clearly broken, and may not work at all (I didn't think so, but maybe the ABI lines up).

  2. A few changed constants, only semi-concerning ones are:

    • MACH_RCV_NOTIFY was 0x200 but the header has it as 0, and 0x200 is unused.
    • MACH_RCV_OVERWRITE was 0x1000 but header has it as 0, and 0x1000 is used by MACH_RCV_GUARDED_DESC. (Unsure if this difference matters but it is a difference)
  3. Stuff like thread_state but it seems so obviously wrong (clearly x86) that it wouldn't be an issue. Actually, I take this back because thread_state gets put into other structures (like context types), and people may only check on x86, so this could be bad.

  4. Missing #[repr(C, packed(4))], but seemingly not in ways that impact layout (I guess these are already align==4, although IMO these should still have the annotation since they do in C).

Of these 1 and 3 are the only really bad ones (and maybe 2), and even 1 may be mostly ABI compatible (although I honestly dunno what happens if you call these with weird types since they aren't quite normal C functions). 3 is pretty bad though, if people don't notice.

Either way, I guess either 1 or 3 would be enough for most -sys crates to get a real advisory.

Let's wait to see if we hear from @fitzgen though, since they might prefer we not do that while they're the maintainer (since it tends to cause a flood of issues, at least if there's no fix available).

Did you see anything else that's wrong in the updates you made for mach2?

@fitzgen
Copy link
Contributor

fitzgen commented Sep 7, 2022

I haven't maintained mach for quite some number of years, @gnzlbg has been the maintainer.

@JohnTitor
Copy link
Owner

Did you see anything else that's wrong in the updates you made for mach2?

No, I've passively-maintained this crate for the libc crate and most contributions are made by other contributors.

I haven't maintained mach for quite some number of years, @gnzlbg has been the maintainer.

As I mentioned in fitzgen/mach#63 (comment), @gnzlbg is now out of contact and they haven't maintained any crates, including libc, ctest, mach, etc. That's why I mentioned you initially. Even if you don't maintain it anymore, you still have access to the repo and crates.io, and you could deprecate the crate (open an issue on rustsec advisory) or transfer the ownership.

@JohnTitor
Copy link
Owner

Let's say this can replace/deprecate mach, actually the rustsec advisory shows this as an alternative.

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

No branches or pull requests

3 participants