Skip to content

[libc-0.2] posix_spawn_file_actions_t cannot be used on Linux after #3602 #3608

Closed
@japaric

Description

@japaric

We found an issue when updating ferrocene's libc submodule (ferrocene/ferrocene#356) to revision 947a185 . the only change included in our libc update was PR #3602 .

when building stage 2 of libstd to x86_64-linux the build failed to execute the build script of the quote crate with the following error:

thread 'main' panicked at library/std/src/sys_common/process.rs:155:17:
called Result::unwrap() on an Err value: Os { code: 9, kind: Uncategorized, message: "Bad file descriptor" } 

where the mentioned line corresponds to this revision.

Tracking down the error led us to this usage of the posix_spawn API:

let mut file_actions = MaybeUninit::uninit();
cvt_nz(libc::posix_spawn_file_actions_init(file_actions.as_mut_ptr()))?;

This code allocates a posix_spawn_file_actions_init type on the stack. before #3602, the file_actions stack variable had the correct size and alignment but after #3602, it now has a size and alignment of *mut c_void (8 bytes on x86_64) and that results in UB when posix_spawn_file_actions_init is called.

GLIBC implements posix_spawn_file_actions_init as a memset operation that zeroes the struct. In Rust syntax, that would be more or less this code:

#[repr(C)]
struct posix_spawn_file_actions_t {
    // fields and padding here
}

unsafe fn posix_spawn_file_actions_init(actions: *mut posix_spawn_file_actions_t) -> c_int {
    ptr::write_bytes(actions, 0, 1);
    0
}

Bionic implements posix_spawn_file_actions_init differently. It uses a heap allocation as indirection. The Rust version of the bionic implementation looks roughly like this:

#[repr(C)]
struct __actual_posix_spawn_file_actions_t {
    // fields and padding here
}

type posix_spawn_file_actions_t = *mut c_void;

unsafe fn posix_spawn_file_actions_init(actions: *mut posix_spawn_file_actions_t) -> c_int {
    let mut alloc = Box::new(MaybeUninit::<__actual_posix_spawn_file_actions_t>::uninit());
    ptr::write_bytes(alloc.as_mut_ptr(), 0, 1);
    *actions = Box::into_raw(alloc).cast();
    0
}

This usage of posix_spawn_file_actions_t in libstd:

let mut file_actions = MaybeUninit::uninit();
cvt_nz(libc::posix_spawn_file_actions_init(file_actions.as_mut_ptr()))?;

is compatible with both GLIBC and Bionic but the libc crate needs to provide the correct size and alignment on Linux.

So, the conditional code should produce this on Linux

pub struct posix_spawn_file_actions_t {
    __allocated: ::c_int,
    __used: ::c_int,
    __actions: *mut ::c_int,
    __pad: [::c_int; 16],
}

but this on Android

pub type posix_spawn_file_actions_t = *mut ::c_void;

All this probably also applies to the posix_spawnattr_t type but I have yet to look into the details of the GLIBC code around that type.

I'll send up a fix PR after I have done some more testing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions