nix icon indicating copy to clipboard operation
nix copied to clipboard

`usize as c_int` is not a conversion that is guaranteed to be correct

Open zacknewman opened this issue 3 months ago • 2 comments

I haven't looked much in the crate, but there is at least one case where usize as c_int is being used. Seeing how c_int is normally an i32 but is technically any signed integer at least the size of c_short, shouldn't c_int::try_from be used instead? For example, unistd::setgroups:

#[cfg(not(any(
    apple_targets,
    target_os = "redox",
    target_os = "haiku"
)))]
pub fn setgroups(groups: &[Gid]) -> Result<()> {
    cfg_if! {
        if #[cfg(any(bsd,
                     solarish,
                     target_os = "aix",
                     target_os = "cygwin"))] {
            type setgroups_ngroups_t = c_int;
        } else {
            type setgroups_ngroups_t = size_t;
        }
    }
    // FIXME: On the platforms we currently support, the `Gid` struct has the
    // same representation in memory as a bare `gid_t`. This is not necessarily
    // the case on all Rust platforms, though. See RFC 1785.
    let res = unsafe {
        libc::setgroups(
            groups.len() as setgroups_ngroups_t,
            groups.as_ptr().cast(),
        )
    };

    Errno::result(res).map(drop)
}

When setgroups_ngroups_t is c_int, groups.len() as setgroups_ngroups_t may produce the wrong result. I'm sure this is unlikely, but it is technically possible.

zacknewman avatar Aug 29 '25 20:08 zacknewman

Yes, that could technically produce the wrong result if the user supplies a slice with billions of groups. Or, if he finds some kind of 16-bit platform that supports Rust.

asomers avatar Sep 13 '25 21:09 asomers

Yes, that could technically produce the wrong result if the user supplies a slice with billions of groups. Or, if he finds some kind of 16-bit platform that supports Rust.

Well it depends on what additional assumptions you want to make that are not provided by Rust or C. Typically in Rust a usize should at most be assumed to be at least a u16 even if it's rare to encounter a situation when it'll be a u16. Additionally C only guarantees that c_int is at least the size of c_short without additional assumptions of the size of the pointer (e.g., it's technically possible for a 64-bit platform to define a C int as a 16-bit integer).

There are 16-bit platforms supported by Rust even if nix doesn't intend to support them (e.g., msp430-none-elf). Feel free to close this if this is a level of pedantry nix doesn't care for. I was writing my own FFI for setgroups and was curious if other crates were as pedantic as me which is why I opened this. I understand not caring about such things. I thought I'd raise an issue just in case.

zacknewman avatar Sep 13 '25 23:09 zacknewman