Future bit flag extensions can cause program failure

[drmikehenry]

I'd like to set a file descriptor to non-blocking mode using nix. In C, I might write something like the below:

#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int
f_getfl(int fd)
{
    int flags = fcntl(fd, F_GETFL);
    return flags;
}

int
f_setfl(int fd, int flags)
{
    int rv = fcntl(fd, F_SETFL, flags);
    return rv;
}

int
main()
{
    int rv;
    int fd = 0;
    char buf[1];
    int flags = f_getfl(fd);
    if (flags == -1)
    {
        perror("f_getfl");
        exit(1);
    }

    rv = f_setfl(fd, flags | O_NONBLOCK);
    if (flags == -1)
    {
        perror("f_setfl");
        exit(1);
    }

    printf("stdin is now non-blocking\n");

    /* Read stdin;  if would block, fail with EAGAIN. */
    rv = read(fd, buf, sizeof(buf));
    if (rv < 0)
    {
        perror("read");
        exit(1);
    }

    printf("read was successful\n");
    return 0;
}

The general procedure is typical: read the original flags, bitwise-OR with some known flag pattern, and write back the flags. This has the important property of preserving any flags already set for the file descriptor. The C code need not be concerned with the specific meaning of any flag value beyond the one it's trying to set (O_NONBLOCK in this case); any flags already set on the file descriptor will be preserved by the bitwise-OR operation. After the C program has been shipped, it's fine for the system to define additional flag values; the program will continue to work correctly even though those flags weren't defined when the program was compiled.

Below is an attempt to write the above program in Rust using nix:

use nix;
use std::os::unix::io::RawFd;

fn f_getfl(fd: RawFd) -> nix::Result<nix::fcntl::OFlag> {
    let bits = nix::fcntl::fcntl(fd, nix::fcntl::F_GETFL)?;

    let flags = nix::fcntl::OFlag::from_bits_truncate(bits);

    Ok(flags)
}

fn f_setfl(fd: RawFd, flags: nix::fcntl::OFlag) -> nix::Result<()> {
    let _ = nix::fcntl::fcntl(fd, nix::fcntl::F_SETFL(flags))?;
    Ok(())
}

fn main() -> nix::Result<()> {
    let fd = 0;
    f_setfl(fd, f_getfl(fd)? | nix::fcntl::OFlag::O_NONBLOCK)?;
    println!("stdin is now non-blocking");

    // Read stdin; if would block, fail with EAGAIN.
    let mut buf: [u8; 1] = [0; 1];
    let _ = nix::unistd::read(fd, &mut buf)?;
    println!("Read was successful");
    Ok(())
}

This has a flaw that the C version doesn't. The flags returned from the operating system must be converted into the type nix::fcntl::OFlag in order to write them back. This type is implemented using the "bitflags" crate which does not permit unknown bit mask value to be stored. It offers only two options for converting an integer bit mask into a bitflags instance:

• from_bits_truncate(), which throws away any unknown flag values.
• from_bits(), which returns an error if any unknown flag values are found.

As written above using from_bits_truncate(), the program will silently corrupt any flags that weren't known to nix at compile time. It's possible to assign flags using from_bits() as follows:

let flags = nix::fcntl::OFlag::from_bits(bits)
    .ok_or(nix::errno::Errno::EINVAL)?;

Now any undefined flags will generate an error instead of silently corrupting the flags, but the desired flag modification is instead forced to fail.

Neither of these options is as future-proof as the standard way of handling flags in C. The problem stems from insisting that all possible bit flag values for all time be known in advance to the nix crate, disallowing the possibility of backward-compatible extension of the flag definitions by the system.

One hack around this problem is to force the bits into the structure:

let flags : nix::fcntl::OFlag = unsafe { std::mem::transmute(bits) };

This properly handles backward-compatible extensions to the bit flags, but such a hack shouldn't be required to regain the C program's robustness.

Because the underlying restriction is built into the bitflags crate, I've submitted a pull request asking for a third method to convert bits into a bitflags structure. This new function, from_bits_unknown(), behaves like from_bits_truncate() except any unknown bits are preserved instead of being truncated. With this change, it's possible to use the nix crate to write robust implementations of f_getfl() and f_setfl() analogous to those in the C program. In fact, I think it would be useful for the nix crate to expose such higher-level type-safe functions directly, once they can be written to account for bit flag additions by the system.

The pull request is here: https://github.com/bitflags/bitflags/pull/188

[asomers]

Thanks for looking into this. It's certainly a real problem, and I think the solution you proposed is a good one. I'll take another look once the bitflags PR gets resolved.

[drmikehenry]

@asomers The bitflags PR was accepted, and bitflags 1.2.0 now has a from_bits_unchecked() function, which is sufficient for my needs. I'll leave it to you to decide whether you're interested in adding typesafe functions such as f_getfl() and f_setfl() into the nix crate, but feel free to close this ticket if that's not a priority for you (or if you'd like to track the work in a different ticket).