Re: [PATCH v16 1/3] fs: Add trusted_for(2) syscall implementation and related sysctl

On 13/11/2021 20:56, Alejandro Colomar (man-pages) wrote:

Hi Mickaël,

On 11/13/21 14:02, Mickaël Salaün wrote:

quoted

quoted

TL;DR:

ISO C specifies that for the following code:

     enum foo {BAR};

     enum foo foobar;

typeof(foo)    shall be int
typeof(foobar) is implementation-defined

I tested with some version of GCC (from 4.9 to 11) and clang (10 and 11)
with different optimizations and the related sizes are at least the same
as for the int type.

GCC has -fshort-enums to make enum types be as short as possible.  I
expected -Os to turn this on, since it saves space, but it doesn't.

Still, not relying on enum == int is better, IMO.

quoted

quoted

Since foobar = BAR; assigns an int, the best thing to do to avoid
implementation-defined behavior, is to declare foobar as int too.

OK, so it should be enough to change the syscall argument type from enum
trusted_for_usage to int, but we can keep the UAPI with the enum (i.e.
we don't need to change the value to #define TRUSTED_FOR_EXECUTION 1)
right?

Correct.  The enumerations are guaranteed to be int (except in case of
UB, see below), so they'll be (almost) the same as a #define after the
preprocessor.

Thanks for the detailed explanation! I'll send a new patch taking into
account your suggestion.

If you do

enum foo {
    FOO = 1L << INT_WIDTH
};

since that doesn't fit in either int or unsigned int,
it is Undefined Behavior,
and here GCC decides to use long for FOO.

+++++++++ UB example ++++++++++++++

$ cat foo.c
    #include <limits.h>
    #include <stdio.h>


    enum foo {
        FOO = 1L << UINT_WIDTH
    };

    int main(void)
    {
        printf("\tsizeof(enum foo) = %zu\n", sizeof(enum foo));
        printf("\tsizeof(FOO)      = %zu\n", sizeof(FOO));
    }

$ cc foo.c -Wall -Wextra -Werror -Wpedantic -pedantic-errors -std=c2x
foo.c:6:23: error: ISO C restricts enumerator values to range of 'int'
[-Wpedantic]
    6 |                 FOO = 1L << UINT_WIDTH
      |                       ^~
$ cc foo.c -Wall -Wextra -Werror -std=c2x
$ ./a.out
    sizeof(enum foo) = 8
    sizeof(FOO)      = 8

+++++++++++++ -fshort-enums example +++++++++++++++

$ cat foo.c
    #include <stdio.h>


    enum foo {
        FOO = 1
    };

    int main(void)
    {
        printf("\tsizeof(enum foo) = %zu\n", sizeof(enum foo));
        printf("\tsizeof(FOO)      = %zu\n", sizeof(FOO));
    }

$ cc foo.c -Wall -Wextra -Werror -Wpedantic -pedantic-errors -fshort-enums
$ ./a.out
    sizeof(enum foo) = 1
    sizeof(FOO)      = 4

++++++++++++++++++++++++++++++++++++++++++++++++++++++

Cheers,
Alex

quoted

quoted

quoted

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 528a478dbda8..c535e0e43cc8 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h

@@ -462,6 +463,7 @@ asmlinkage long sys_fallocate(int fd, int mode,

loff_t offset, loff_t len);
   asmlinkage long sys_faccessat(int dfd, const char __user *filename,
int mode);
   asmlinkage long sys_faccessat2(int dfd, const char __user *filename,
int mode,
                      int flags);
+asmlinkage long sys_trusted_for(int fd, enum trusted_for_usage usage,
u32 flags);

Same here.

quoted

   asmlinkage long sys_chdir(const char __user *filename);
   asmlinkage long sys_fchdir(unsigned int fd);
   asmlinkage long sys_chroot(const char __user *filename);

Thanks,
Alex