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

Group::from_name misaligned pointer dereference #2304

Closed
anonkey opened this issue Feb 1, 2024 · 6 comments · Fixed by #2311
Closed

Group::from_name misaligned pointer dereference #2304

anonkey opened this issue Feb 1, 2024 · 6 comments · Fixed by #2311

Comments

@anonkey
Copy link
Contributor

anonkey commented Feb 1, 2024

Hello,
I'm on MacOS Sonoma on M1 processor

I've a panic only happening in dev not in release on this function call :

 let group = Group::from_name(group)?;

Result:

thread 'main' panicked at /Users/key/.cargo/registry/src/index.crates.io-6f17d22bba15001f/nix-0.27.1/src/unistd.rs:3794:16:
misaligned pointer dereference: address must be a multiple of 0x8 but is 0x14f809a09  

I tried some flags : -C overflow-checks -C debug-assertions=on

But nothing change i can't figure out what's happening

Have any clue ?

Thanks

@asomers
Copy link
Member

asomers commented Feb 1, 2024

It's a bug in Nix. Try this patch (against the master branch, not v0.27.1 )

diff --cc src/unistd.rs
index 3d98a0efe,8c23d7c38..000000000
--- a/src/unistd.rs
+++ b/src/unistd.rs
@@@ -3476,11 -3790,11 +3476,11 @@@ impl Group 
          let mut ret = Vec::new();
  
          for i in 0.. {
-             let u = unsafe { mem.offset(i) };
-             if unsafe { (*u).is_null() } {
 -            let u: *mut *mut c_char = mem.offset(i);
 -            if (*u).is_null() {
++            let u = unsafe { mem.offset(i).read_unaligned() };
++            if u.is_null() {
                  break;
              } else {
-                 let s = unsafe {CStr::from_ptr(*u).to_string_lossy().into_owned()};
 -                let s = CStr::from_ptr(*u).to_string_lossy().into_owned();
++                let s = unsafe {CStr::from_ptr(u).to_string_lossy().into_owned()};
                  ret.push(s);
              }
          }

@SteveLauC
Copy link
Member

If this is a known bug and we know how to fix it, why not file a PR to get it resolved?

@anonkey
Copy link
Contributor Author

anonkey commented Feb 6, 2024

If this is a known bug and we know how to fix it, why not file a PR to get it resolved?

I was wondering why too.

I tried to apply the fix but since i have some crates who uses another version it seems to not be compatible, i'll dig further asap

@SteveLauC
Copy link
Member

We do have a test for this function, but it is only enabled on Linux.

I am thinking about adding a test for Group::from_gid() and Group::from_name() through the wheel group on macOS and possibly other BSDs? (As this group came from BSD)

    use nix::unistd::Group;

    let wheel = Group::from_name("wheel").unwrap().unwrap();
    assert_eq!(wheel.name, "wheel");
    let wheel_id = wheel.gid;

    let wheel = Group::from_gid(wheel_id).unwrap().unwrap();
    assert_eq!(wheel.gid, wheel_id);
    assert_eq!(wheel.name, "wheel");

Unless it is removed deliberately, this group should exist.

@asomers
Copy link
Member

asomers commented Feb 6, 2024

If this is a known bug and we know how to fix it, why not file a PR to get it resolved?

I mean that it's a known bug now, now that @anonkey reported it.

@SteveLauC
Copy link
Member

I mean that it's a known bug now, now that @anonkey reported it.

Ohh, I am sorry that I misunderstood it

@SteveLauC SteveLauC linked a pull request Feb 16, 2024 that will close this issue
3 tasks
grahamc added a commit to DeterminateSystems/nix-installer that referenced this issue Apr 1, 2024
grahamc added a commit to DeterminateSystems/nix-installer that referenced this issue Apr 11, 2024
cole-h pushed a commit to DeterminateSystems/nix-installer that referenced this issue Apr 15, 2024
* Drop riff support

* Relocate the darwin diskutil structs to make room for keychain

* Clean up a few warnings

* Initial support for Determinate Nix Enterprise

* Don't create the org.nixos.darwin-store service under enterprise nix.

* Only write out systems.determinate.nix-daemon under nix enterprise

* Bump the nix crate from 0.27.0 to 0.28.0 for nix-rust/nix#2304

* Fixup the test fixture for macos

* Correct ConfigureInitService on Linux

* Fixup fixtures for linux

* Use Determinate Nix for macOS to perform the mount during installation.

* Force-enable encryption for Nix Enterprise on macOS

* fixup

* drop extraneous comment

* Create a separate CreateNixEnterpriseVolume to simplify CreateNixVolume.

* fixup

* Relocate the nix-enterprise flag down into the macOS planner, since our planning requires no changes on other targets at the moment.

* Fail the install if --nix-enterprise is passed but isn't available

* fixup nix_enterprise bits

* I hate this.

* Rename to be Determinate Nix Enterprise Edition, to be more consistent across tools and products.

* Note why we match on enterprise edition in the macos planner for the otherwise no-op enterprise_editition encryption case

* fixup

* Factor out the enterprise init service setup into its own module

* Conditionally import / use EE init service

* nit

* derp

* fixup: too many args to plan

* ...derp...
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

Successfully merging a pull request may close this issue.

3 participants