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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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/



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux