Messages in this thread Patch in this message | | | Date | Fri, 12 Nov 2021 20:16:01 +0100 | Subject | Re: [PATCH v16 1/3] fs: Add trusted_for(2) syscall implementation and related sysctl | From | "Alejandro Colomar (man-pages)" <> |
| |
Hi Mickaël,
On 11/10/21 20:06, Mickaël Salaün wrote: > diff --git a/fs/open.c b/fs/open.c > index f732fb94600c..96a80abec41b 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -480,6 +482,114 @@ SYSCALL_DEFINE2(access, const char __user *, filename, int, mode) > return do_faccessat(AT_FDCWD, filename, mode, 0); > } > > +#define TRUST_POLICY_EXEC_MOUNT BIT(0) > +#define TRUST_POLICY_EXEC_FILE BIT(1) > + > +int sysctl_trusted_for_policy __read_mostly; > + > +/** ... > + */ > +SYSCALL_DEFINE3(trusted_for, const int, fd, const enum trusted_for_usage, usage,
Please, don't use enums for interfaces. They are implementation defined types, and vary between compilers and within the same compiler also depending on optimization flags.
C17::6.7.2.2.4: [ Each enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type. The choice of type is implementation-defined,130) but shall be capable of representing the values of all the members of the enumeration. ]
See also: <https://stackoverflow.com/questions/366017/what-is-the-size-of-an-enum-in-c>
So, please use only standard integer types for interfaces.
And in the case of enums, since the language specifies that enumeration constants (the macro-like identifiers) are of type int, it makes sense for functions to use int.
C17::6.7.2.2.3: [ The identifiers in an enumerator list are declared as constants that have type int and may appear wherever such are permitted. ]
I'd use an int for the API/ABI, even if it's expected to be assigned values of 'enum trusted_for_usage' (that should be specified in the manual page in DESCRIPTION, but not in SYNOPSIS, which should specify int).
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
Since foobar = BAR; assigns an int, the best thing to do to avoid implementation-defined behavior, is to declare foobar as int too.
> 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.
> 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
-- Alejandro Colomar Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/ -- Alejandro Colomar Linux man-pages comaintainer; http://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/
| |