[loeg]

> systemd was using a magic value (-1) to represent an invalid mode_t value, and C's type system did not prevent passing it to the mode argument of open

The open syscall should reject unrecognized flags in the mode argument (EINVAL), rather than just truncating down to recognized flags. That would also prevent this specific problem with the sentinel value being used on accident.

[JdeBP]

In the actual open and openat system calls, the mode parameter is a 16-bit unsigned integer. There are 12 permissions bits and 4 file type bits. There are no "unrecognized flags". All 16 bits have meaning.

* http://lxr.free-electrons.com/source/include/uapi/linux/stat...

* http://lxr.free-electrons.com/source/include/linux/types.h#L...

[loeg]

Bits above 1<<11 (the non-permission bits) are not valid arguments to open(2), so I don't see what your point is. In this context, they are invalid. open(2) should reject values with bits outside of 07777 (== 0x0fff) set, including (mode_t)-1 (== 0xffff).

Here is the specific place where Linux truncates the bogus mode, instead of rejecting it: http://lxr.free-electrons.com/source/fs/open.c#L906

(S_IALLUGO defined here: http://lxr.free-electrons.com/source/include/linux/stat.h#L9 )

This change would fix this class of issue:

    --- a/fs/open.c
    +++ b/fs/open.c
    @@ -889,9 +889,11 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
            int lookup_flags = 0;
            int acc_mode = ACC_MODE(flags);
    
    -       if (flags & (O_CREAT | __O_TMPFILE))
    +       if (flags & (O_CREAT | __O_TMPFILE)) {
    +               if ((mode & ~S_IALLUGO) != 0)
    +                       return -EINVAL;
                    op->mode = (mode & S_IALLUGO) | S_IFREG;
    -       else
    +       } else
                    op->mode = 0;
    
            /* Must never be set by userspace */