[v1,6/8] LSM: lsm_self_attr syscall for LSM self attributes

  Create a system call lsm_self_attr() to provide the security
module maintained attributes of the current process. Historically
these attributes have been exposed to user space via entries in
procfs under /proc/self/attr.

Attributes are provided as a collection of lsm_ctx structures
which are placed into a user supplied buffer. Each structure
identifys the security module providing the attribute, which
of the possible attributes is provided, the size of the
attribute, and finally the attribute value. The format of the
attribute value is defined by the security module, but will
always be \0 terminated. The ctx_len value will be larger than
strlen(ctx).

        ------------------------------
        | unsigned int id            |
        ------------------------------
        | unsigned int flags         |
        ------------------------------
        | __kernel_size_t ctx_len    |
        ------------------------------
        | unsigned char ctx[ctx_len] |
        ------------------------------
        | unsigned int id            |
        ------------------------------
        | unsigned int flags         |
        ------------------------------
        | __kernel_size_t ctx_len    |
        ------------------------------
        | unsigned char ctx[ctx_len] |
        ------------------------------

Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
 include/linux/syscalls.h |   2 +
 include/uapi/linux/lsm.h |  21 ++++++
 kernel/sys_ni.c          |   3 +
 security/Makefile        |   1 +
 security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 183 insertions(+)
 create mode 100644 security/lsm_syscalls.c
  

Hi Casey,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kees/for-next/hardening]
[also build test WARNING on pcmoore-selinux/next acme/perf/core linus/master v6.1-rc2 next-20221025]
[cannot apply to tip/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
patch link:    https://lore.kernel.org/r/20221025184519.13231-7-casey%40schaufler-ca.com
patch subject: [PATCH v1 6/8] LSM: lsm_self_attr syscall for LSM self attributes
config: ia64-allyesconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/c9d17b230f202246a9451fbdefac8c1720eb68a6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
        git checkout c9d17b230f202246a9451fbdefac8c1720eb68a6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash kernel/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from kernel/fork.c:55:
>> include/linux/syscalls.h:1060:42: warning: 'struct lsm_ctx' declared inside parameter list will not be visible outside of this definition or declaration
    1060 | asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
         |                                          ^~~~~~~
   kernel/fork.c:162:13: warning: no previous prototype for 'arch_release_task_struct' [-Wmissing-prototypes]
     162 | void __weak arch_release_task_struct(struct task_struct *tsk)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~
   kernel/fork.c:849:20: warning: no previous prototype for 'arch_task_cache_init' [-Wmissing-prototypes]
     849 | void __init __weak arch_task_cache_init(void) { }
         |                    ^~~~~~~~~~~~~~~~~~~~
   kernel/fork.c:944:12: warning: no previous prototype for 'arch_dup_task_struct' [-Wmissing-prototypes]
     944 | int __weak arch_dup_task_struct(struct task_struct *dst,
         |            ^~~~~~~~~~~~~~~~~~~~
--
   In file included from kernel/exec_domain.c:19:
>> include/linux/syscalls.h:1060:42: warning: 'struct lsm_ctx' declared inside parameter list will not be visible outside of this definition or declaration
    1060 | asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
         |                                          ^~~~~~~
--
   In file included from kernel/exit.c:42:
>> include/linux/syscalls.h:1060:42: warning: 'struct lsm_ctx' declared inside parameter list will not be visible outside of this definition or declaration
    1060 | asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
         |                                          ^~~~~~~
   kernel/exit.c:1839:13: warning: no previous prototype for 'abort' [-Wmissing-prototypes]
    1839 | __weak void abort(void)
         |             ^~~~~
--
   In file included from kernel/audit.c:44:
>> include/linux/syscalls.h:1060:42: warning: 'struct lsm_ctx' declared inside parameter list will not be visible outside of this definition or declaration
    1060 | asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
         |                                          ^~~~~~~
   kernel/audit.c: In function 'audit_log_vformat':
   kernel/audit.c:1963:9: warning: function 'audit_log_vformat' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    1963 |         len = vsnprintf(skb_tail_pointer(skb), avail, fmt, args);
         |         ^~~
   kernel/audit.c:1972:17: warning: function 'audit_log_vformat' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    1972 |                 len = vsnprintf(skb_tail_pointer(skb), avail, fmt, args2);
         |                 ^~~
--
   In file included from include/linux/syscalls_api.h:1,
                    from kernel/sched/sched.h:61,
                    from kernel/sched/fair.c:55:
>> include/linux/syscalls.h:1060:42: warning: 'struct lsm_ctx' declared inside parameter list will not be visible outside of this definition or declaration
    1060 | asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
         |                                          ^~~~~~~
   kernel/sched/fair.c:11509:6: warning: no previous prototype for 'task_vruntime_update' [-Wmissing-prototypes]
   11509 | void task_vruntime_update(struct rq *rq, struct task_struct *p, bool in_fi)
         |      ^~~~~~~~~~~~~~~~~~~~
--
   In file included from kernel/time/hrtimer.c:30:
>> include/linux/syscalls.h:1060:42: warning: 'struct lsm_ctx' declared inside parameter list will not be visible outside of this definition or declaration
    1060 | asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
         |                                          ^~~~~~~
   kernel/time/hrtimer.c:120:35: warning: initialized field overwritten [-Woverride-init]
     120 |         [CLOCK_REALTIME]        = HRTIMER_BASE_REALTIME,
         |                                   ^~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:120:35: note: (near initialization for 'hrtimer_clock_to_base_table[0]')
   kernel/time/hrtimer.c:121:35: warning: initialized field overwritten [-Woverride-init]
     121 |         [CLOCK_MONOTONIC]       = HRTIMER_BASE_MONOTONIC,
         |                                   ^~~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:121:35: note: (near initialization for 'hrtimer_clock_to_base_table[1]')
   kernel/time/hrtimer.c:122:35: warning: initialized field overwritten [-Woverride-init]
     122 |         [CLOCK_BOOTTIME]        = HRTIMER_BASE_BOOTTIME,
         |                                   ^~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:122:35: note: (near initialization for 'hrtimer_clock_to_base_table[7]')
   kernel/time/hrtimer.c:123:35: warning: initialized field overwritten [-Woverride-init]
     123 |         [CLOCK_TAI]             = HRTIMER_BASE_TAI,
         |                                   ^~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:123:35: note: (near initialization for 'hrtimer_clock_to_base_table[11]')
   kernel/time/hrtimer.c: In function '__run_hrtimer':
   kernel/time/hrtimer.c:1648:14: warning: variable 'expires_in_hardirq' set but not used [-Wunused-but-set-variable]
    1648 |         bool expires_in_hardirq;
         |              ^~~~~~~~~~~~~~~~~~


vim +1060 include/linux/syscalls.h

   904	
   905	/* mm/, CONFIG_MMU only */
   906	asmlinkage long sys_swapon(const char __user *specialfile, int swap_flags);
   907	asmlinkage long sys_swapoff(const char __user *specialfile);
   908	asmlinkage long sys_mprotect(unsigned long start, size_t len,
   909					unsigned long prot);
   910	asmlinkage long sys_msync(unsigned long start, size_t len, int flags);
   911	asmlinkage long sys_mlock(unsigned long start, size_t len);
   912	asmlinkage long sys_munlock(unsigned long start, size_t len);
   913	asmlinkage long sys_mlockall(int flags);
   914	asmlinkage long sys_munlockall(void);
   915	asmlinkage long sys_mincore(unsigned long start, size_t len,
   916					unsigned char __user * vec);
   917	asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
   918	asmlinkage long sys_process_madvise(int pidfd, const struct iovec __user *vec,
   919				size_t vlen, int behavior, unsigned int flags);
   920	asmlinkage long sys_process_mrelease(int pidfd, unsigned int flags);
   921	asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
   922				unsigned long prot, unsigned long pgoff,
   923				unsigned long flags);
   924	asmlinkage long sys_mbind(unsigned long start, unsigned long len,
   925					unsigned long mode,
   926					const unsigned long __user *nmask,
   927					unsigned long maxnode,
   928					unsigned flags);
   929	asmlinkage long sys_get_mempolicy(int __user *policy,
   930					unsigned long __user *nmask,
   931					unsigned long maxnode,
   932					unsigned long addr, unsigned long flags);
   933	asmlinkage long sys_set_mempolicy(int mode, const unsigned long __user *nmask,
   934					unsigned long maxnode);
   935	asmlinkage long sys_migrate_pages(pid_t pid, unsigned long maxnode,
   936					const unsigned long __user *from,
   937					const unsigned long __user *to);
   938	asmlinkage long sys_move_pages(pid_t pid, unsigned long nr_pages,
   939					const void __user * __user *pages,
   940					const int __user *nodes,
   941					int __user *status,
   942					int flags);
   943	
   944	asmlinkage long sys_rt_tgsigqueueinfo(pid_t tgid, pid_t  pid, int sig,
   945			siginfo_t __user *uinfo);
   946	asmlinkage long sys_perf_event_open(
   947			struct perf_event_attr __user *attr_uptr,
   948			pid_t pid, int cpu, int group_fd, unsigned long flags);
   949	asmlinkage long sys_accept4(int, struct sockaddr __user *, int __user *, int);
   950	asmlinkage long sys_recvmmsg(int fd, struct mmsghdr __user *msg,
   951				     unsigned int vlen, unsigned flags,
   952				     struct __kernel_timespec __user *timeout);
   953	asmlinkage long sys_recvmmsg_time32(int fd, struct mmsghdr __user *msg,
   954				     unsigned int vlen, unsigned flags,
   955				     struct old_timespec32 __user *timeout);
   956	
   957	asmlinkage long sys_wait4(pid_t pid, int __user *stat_addr,
   958					int options, struct rusage __user *ru);
   959	asmlinkage long sys_prlimit64(pid_t pid, unsigned int resource,
   960					const struct rlimit64 __user *new_rlim,
   961					struct rlimit64 __user *old_rlim);
   962	asmlinkage long sys_fanotify_init(unsigned int flags, unsigned int event_f_flags);
   963	asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
   964					  u64 mask, int fd,
   965					  const char  __user *pathname);
   966	asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
   967					      struct file_handle __user *handle,
   968					      int __user *mnt_id, int flag);
   969	asmlinkage long sys_open_by_handle_at(int mountdirfd,
   970					      struct file_handle __user *handle,
   971					      int flags);
   972	asmlinkage long sys_clock_adjtime(clockid_t which_clock,
   973					struct __kernel_timex __user *tx);
   974	asmlinkage long sys_clock_adjtime32(clockid_t which_clock,
   975					struct old_timex32 __user *tx);
   976	asmlinkage long sys_syncfs(int fd);
   977	asmlinkage long sys_setns(int fd, int nstype);
   978	asmlinkage long sys_pidfd_open(pid_t pid, unsigned int flags);
   979	asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
   980				     unsigned int vlen, unsigned flags);
   981	asmlinkage long sys_process_vm_readv(pid_t pid,
   982					     const struct iovec __user *lvec,
   983					     unsigned long liovcnt,
   984					     const struct iovec __user *rvec,
   985					     unsigned long riovcnt,
   986					     unsigned long flags);
   987	asmlinkage long sys_process_vm_writev(pid_t pid,
   988					      const struct iovec __user *lvec,
   989					      unsigned long liovcnt,
   990					      const struct iovec __user *rvec,
   991					      unsigned long riovcnt,
   992					      unsigned long flags);
   993	asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
   994				 unsigned long idx1, unsigned long idx2);
   995	asmlinkage long sys_finit_module(int fd, const char __user *uargs, int flags);
   996	asmlinkage long sys_sched_setattr(pid_t pid,
   997						struct sched_attr __user *attr,
   998						unsigned int flags);
   999	asmlinkage long sys_sched_getattr(pid_t pid,
  1000						struct sched_attr __user *attr,
  1001						unsigned int size,
  1002						unsigned int flags);
  1003	asmlinkage long sys_renameat2(int olddfd, const char __user *oldname,
  1004				      int newdfd, const char __user *newname,
  1005				      unsigned int flags);
  1006	asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
  1007				    void __user *uargs);
  1008	asmlinkage long sys_getrandom(char __user *buf, size_t count,
  1009				      unsigned int flags);
  1010	asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
  1011	asmlinkage long sys_bpf(int cmd, union bpf_attr *attr, unsigned int size);
  1012	asmlinkage long sys_execveat(int dfd, const char __user *filename,
  1013				const char __user *const __user *argv,
  1014				const char __user *const __user *envp, int flags);
  1015	asmlinkage long sys_userfaultfd(int flags);
  1016	asmlinkage long sys_membarrier(int cmd, unsigned int flags, int cpu_id);
  1017	asmlinkage long sys_mlock2(unsigned long start, size_t len, int flags);
  1018	asmlinkage long sys_copy_file_range(int fd_in, loff_t __user *off_in,
  1019					    int fd_out, loff_t __user *off_out,
  1020					    size_t len, unsigned int flags);
  1021	asmlinkage long sys_preadv2(unsigned long fd, const struct iovec __user *vec,
  1022				    unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
  1023				    rwf_t flags);
  1024	asmlinkage long sys_pwritev2(unsigned long fd, const struct iovec __user *vec,
  1025				    unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
  1026				    rwf_t flags);
  1027	asmlinkage long sys_pkey_mprotect(unsigned long start, size_t len,
  1028					  unsigned long prot, int pkey);
  1029	asmlinkage long sys_pkey_alloc(unsigned long flags, unsigned long init_val);
  1030	asmlinkage long sys_pkey_free(int pkey);
  1031	asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
  1032				  unsigned mask, struct statx __user *buffer);
  1033	asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
  1034				 int flags, uint32_t sig);
  1035	asmlinkage long sys_open_tree(int dfd, const char __user *path, unsigned flags);
  1036	asmlinkage long sys_move_mount(int from_dfd, const char __user *from_path,
  1037				       int to_dfd, const char __user *to_path,
  1038				       unsigned int ms_flags);
  1039	asmlinkage long sys_mount_setattr(int dfd, const char __user *path,
  1040					  unsigned int flags,
  1041					  struct mount_attr __user *uattr, size_t usize);
  1042	asmlinkage long sys_fsopen(const char __user *fs_name, unsigned int flags);
  1043	asmlinkage long sys_fsconfig(int fs_fd, unsigned int cmd, const char __user *key,
  1044				     const void __user *value, int aux);
  1045	asmlinkage long sys_fsmount(int fs_fd, unsigned int flags, unsigned int ms_flags);
  1046	asmlinkage long sys_fspick(int dfd, const char __user *path, unsigned int flags);
  1047	asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
  1048					       siginfo_t __user *info,
  1049					       unsigned int flags);
  1050	asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
  1051	asmlinkage long sys_landlock_create_ruleset(const struct landlock_ruleset_attr __user *attr,
  1052			size_t size, __u32 flags);
  1053	asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum landlock_rule_type rule_type,
  1054			const void __user *rule_attr, __u32 flags);
  1055	asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 flags);
  1056	asmlinkage long sys_memfd_secret(unsigned int flags);
  1057	asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
  1058						    unsigned long home_node,
  1059						    unsigned long flags);
> 1060	asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
  1061
  

On Tue, Oct 25, 2022 at 11:45:17AM -0700, Casey Schaufler wrote:
> Create a system call lsm_self_attr() to provide the security
> module maintained attributes of the current process. Historically
> these attributes have been exposed to user space via entries in
> procfs under /proc/self/attr.
> 
> Attributes are provided as a collection of lsm_ctx structures
> which are placed into a user supplied buffer. Each structure
> identifys the security module providing the attribute, which
> of the possible attributes is provided, the size of the
> attribute, and finally the attribute value. The format of the
> attribute value is defined by the security module, but will
> always be \0 terminated. The ctx_len value will be larger than
> strlen(ctx).
> 
>         ------------------------------
>         | unsigned int id            |
>         ------------------------------
>         | unsigned int flags         |
>         ------------------------------
>         | __kernel_size_t ctx_len    |
>         ------------------------------
>         | unsigned char ctx[ctx_len] |
>         ------------------------------
>         | unsigned int id            |
>         ------------------------------
>         | unsigned int flags         |
>         ------------------------------
>         | __kernel_size_t ctx_len    |
>         ------------------------------
>         | unsigned char ctx[ctx_len] |
>         ------------------------------
> 
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/syscalls.h |   2 +
>  include/uapi/linux/lsm.h |  21 ++++++
>  kernel/sys_ni.c          |   3 +
>  security/Makefile        |   1 +
>  security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 183 insertions(+)
>  create mode 100644 security/lsm_syscalls.c
> 
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index a34b0f9a9972..2d9033e9e5a0 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -71,6 +71,7 @@ struct clone_args;
>  struct open_how;
>  struct mount_attr;
>  struct landlock_ruleset_attr;
> +struct lsm_cxt;
>  enum landlock_rule_type;
>  
>  #include <linux/types.h>
> @@ -1056,6 +1057,7 @@ asmlinkage long sys_memfd_secret(unsigned int flags);
>  asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
>  					    unsigned long home_node,
>  					    unsigned long flags);
> +asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
>  
>  /*
>   * Architecture-specific system calls
> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> index 61e13b1b9ece..1d27fb5b7746 100644
> --- a/include/uapi/linux/lsm.h
> +++ b/include/uapi/linux/lsm.h
> @@ -9,6 +9,27 @@
>  #ifndef _UAPI_LINUX_LSM_H
>  #define _UAPI_LINUX_LSM_H
>  
> +#include <linux/types.h>
> +#include <linux/unistd.h>
> +
> +/**
> + * struct lsm_ctx - LSM context
> + * @id: the LSM id number, see LSM_ID_XXX
> + * @flags: context specifier and LSM specific flags
> + * @ctx_len: the size of @ctx
> + * @ctx: the LSM context, a nul terminated string
> + *
> + * @ctx in a nul terminated string.
> + *	(strlen(@ctx) < @ctx_len) is always true.
> + *	(strlen(@ctx) == @ctx_len + 1) is not guaranteed.
> + */
> +struct lsm_ctx {
> +	unsigned int		id;
> +	unsigned int		flags;
> +	__kernel_size_t		ctx_len;
> +	unsigned char		ctx[];

Please use data types that are allowed to cross the user/kernel boundry
in a safe way.  That would mean this would use __u64 instead of unsigned
int, and __u8 instead of unsigned char.

thanks,

greg k-h
  

Hi Casey,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kees/for-next/hardening]
[also build test WARNING on pcmoore-selinux/next acme/perf/core linus/master v6.1-rc2 next-20221026]
[cannot apply to tip/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
patch link:    https://lore.kernel.org/r/20221025184519.13231-7-casey%40schaufler-ca.com
patch subject: [PATCH v1 6/8] LSM: lsm_self_attr syscall for LSM self attributes
config: arm-randconfig-r031-20221025 (attached as .config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/c9d17b230f202246a9451fbdefac8c1720eb68a6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
        git checkout c9d17b230f202246a9451fbdefac8c1720eb68a6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash kernel/time/ security/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from security/lsm_syscalls.c:15:
>> include/linux/syscalls.h:1060:42: warning: declaration of 'struct lsm_ctx' will not be visible outside of this function [-Wvisibility]
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                                            ^
   security/lsm_syscalls.c:47:1: error: conflicting types for 'sys_lsm_self_attr'
   SYSCALL_DEFINE3(lsm_self_attr,
   ^
   include/linux/syscalls.h:220:36: note: expanded from macro 'SYSCALL_DEFINE3'
   #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                      ^
   include/linux/syscalls.h:229:2: note: expanded from macro 'SYSCALL_DEFINEx'
           __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
           ^
   include/linux/syscalls.h:243:18: note: expanded from macro '__SYSCALL_DEFINEx'
           asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))       \
                           ^
   <scratch space>:65:1: note: expanded from here
   sys_lsm_self_attr
   ^
   include/linux/syscalls.h:1060:17: note: previous declaration is here
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                   ^
   1 warning and 1 error generated.
--
   In file included from kernel/time/time.c:33:
>> include/linux/syscalls.h:1060:42: warning: declaration of 'struct lsm_ctx' will not be visible outside of this function [-Wvisibility]
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                                            ^
   1 warning generated.
--
   In file included from kernel/time/timer.c:35:
>> include/linux/syscalls.h:1060:42: warning: declaration of 'struct lsm_ctx' will not be visible outside of this function [-Wvisibility]
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                                            ^
   kernel/time/timer.c:1365:20: warning: unused function 'del_timer_wait_running' [-Wunused-function]
   static inline void del_timer_wait_running(struct timer_list *timer) { }
                      ^
   2 warnings generated.
--
   In file included from kernel/time/hrtimer.c:30:
>> include/linux/syscalls.h:1060:42: warning: declaration of 'struct lsm_ctx' will not be visible outside of this function [-Wvisibility]
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                                            ^
   kernel/time/hrtimer.c:120:21: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
           [CLOCK_REALTIME]        = HRTIMER_BASE_REALTIME,
                                     ^~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:118:27: note: previous initialization is here
           [0 ... MAX_CLOCKS - 1]  = HRTIMER_MAX_CLOCK_BASES,
                                     ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:121:22: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
           [CLOCK_MONOTONIC]       = HRTIMER_BASE_MONOTONIC,
                                     ^~~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:118:27: note: previous initialization is here
           [0 ... MAX_CLOCKS - 1]  = HRTIMER_MAX_CLOCK_BASES,
                                     ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:122:21: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
           [CLOCK_BOOTTIME]        = HRTIMER_BASE_BOOTTIME,
                                     ^~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:118:27: note: previous initialization is here
           [0 ... MAX_CLOCKS - 1]  = HRTIMER_MAX_CLOCK_BASES,
                                     ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:123:17: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
           [CLOCK_TAI]             = HRTIMER_BASE_TAI,
                                     ^~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:118:27: note: previous initialization is here
           [0 ... MAX_CLOCKS - 1]  = HRTIMER_MAX_CLOCK_BASES,
                                     ^~~~~~~~~~~~~~~~~~~~~~~
   kernel/time/hrtimer.c:1648:7: warning: variable 'expires_in_hardirq' set but not used [-Wunused-but-set-variable]
           bool expires_in_hardirq;
                ^
   kernel/time/hrtimer.c:276:20: warning: unused function 'is_migration_base' [-Wunused-function]
   static inline bool is_migration_base(struct hrtimer_clock_base *base)
                      ^
   kernel/time/hrtimer.c:1873:20: warning: unused function '__hrtimer_peek_ahead_timers' [-Wunused-function]
   static inline void __hrtimer_peek_ahead_timers(void)
                      ^
   8 warnings generated.
--
   In file included from kernel/time/posix-stubs.c:13:
>> include/linux/syscalls.h:1060:42: warning: declaration of 'struct lsm_ctx' will not be visible outside of this function [-Wvisibility]
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                                            ^
   kernel/time/posix-stubs.c:25:17: warning: no previous prototype for function 'sys_ni_posix_timers' [-Wmissing-prototypes]
   asmlinkage long sys_ni_posix_timers(void)
                   ^
   kernel/time/posix-stubs.c:25:12: note: declare 'static' if the function is not intended to be used outside of this translation unit
   asmlinkage long sys_ni_posix_timers(void)
              ^
              static 
   2 warnings generated.


vim +1060 include/linux/syscalls.h

   904	
   905	/* mm/, CONFIG_MMU only */
   906	asmlinkage long sys_swapon(const char __user *specialfile, int swap_flags);
   907	asmlinkage long sys_swapoff(const char __user *specialfile);
   908	asmlinkage long sys_mprotect(unsigned long start, size_t len,
   909					unsigned long prot);
   910	asmlinkage long sys_msync(unsigned long start, size_t len, int flags);
   911	asmlinkage long sys_mlock(unsigned long start, size_t len);
   912	asmlinkage long sys_munlock(unsigned long start, size_t len);
   913	asmlinkage long sys_mlockall(int flags);
   914	asmlinkage long sys_munlockall(void);
   915	asmlinkage long sys_mincore(unsigned long start, size_t len,
   916					unsigned char __user * vec);
   917	asmlinkage long sys_madvise(unsigned long start, size_t len, int behavior);
   918	asmlinkage long sys_process_madvise(int pidfd, const struct iovec __user *vec,
   919				size_t vlen, int behavior, unsigned int flags);
   920	asmlinkage long sys_process_mrelease(int pidfd, unsigned int flags);
   921	asmlinkage long sys_remap_file_pages(unsigned long start, unsigned long size,
   922				unsigned long prot, unsigned long pgoff,
   923				unsigned long flags);
   924	asmlinkage long sys_mbind(unsigned long start, unsigned long len,
   925					unsigned long mode,
   926					const unsigned long __user *nmask,
   927					unsigned long maxnode,
   928					unsigned flags);
   929	asmlinkage long sys_get_mempolicy(int __user *policy,
   930					unsigned long __user *nmask,
   931					unsigned long maxnode,
   932					unsigned long addr, unsigned long flags);
   933	asmlinkage long sys_set_mempolicy(int mode, const unsigned long __user *nmask,
   934					unsigned long maxnode);
   935	asmlinkage long sys_migrate_pages(pid_t pid, unsigned long maxnode,
   936					const unsigned long __user *from,
   937					const unsigned long __user *to);
   938	asmlinkage long sys_move_pages(pid_t pid, unsigned long nr_pages,
   939					const void __user * __user *pages,
   940					const int __user *nodes,
   941					int __user *status,
   942					int flags);
   943	
   944	asmlinkage long sys_rt_tgsigqueueinfo(pid_t tgid, pid_t  pid, int sig,
   945			siginfo_t __user *uinfo);
   946	asmlinkage long sys_perf_event_open(
   947			struct perf_event_attr __user *attr_uptr,
   948			pid_t pid, int cpu, int group_fd, unsigned long flags);
   949	asmlinkage long sys_accept4(int, struct sockaddr __user *, int __user *, int);
   950	asmlinkage long sys_recvmmsg(int fd, struct mmsghdr __user *msg,
   951				     unsigned int vlen, unsigned flags,
   952				     struct __kernel_timespec __user *timeout);
   953	asmlinkage long sys_recvmmsg_time32(int fd, struct mmsghdr __user *msg,
   954				     unsigned int vlen, unsigned flags,
   955				     struct old_timespec32 __user *timeout);
   956	
   957	asmlinkage long sys_wait4(pid_t pid, int __user *stat_addr,
   958					int options, struct rusage __user *ru);
   959	asmlinkage long sys_prlimit64(pid_t pid, unsigned int resource,
   960					const struct rlimit64 __user *new_rlim,
   961					struct rlimit64 __user *old_rlim);
   962	asmlinkage long sys_fanotify_init(unsigned int flags, unsigned int event_f_flags);
   963	asmlinkage long sys_fanotify_mark(int fanotify_fd, unsigned int flags,
   964					  u64 mask, int fd,
   965					  const char  __user *pathname);
   966	asmlinkage long sys_name_to_handle_at(int dfd, const char __user *name,
   967					      struct file_handle __user *handle,
   968					      int __user *mnt_id, int flag);
   969	asmlinkage long sys_open_by_handle_at(int mountdirfd,
   970					      struct file_handle __user *handle,
   971					      int flags);
   972	asmlinkage long sys_clock_adjtime(clockid_t which_clock,
   973					struct __kernel_timex __user *tx);
   974	asmlinkage long sys_clock_adjtime32(clockid_t which_clock,
   975					struct old_timex32 __user *tx);
   976	asmlinkage long sys_syncfs(int fd);
   977	asmlinkage long sys_setns(int fd, int nstype);
   978	asmlinkage long sys_pidfd_open(pid_t pid, unsigned int flags);
   979	asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
   980				     unsigned int vlen, unsigned flags);
   981	asmlinkage long sys_process_vm_readv(pid_t pid,
   982					     const struct iovec __user *lvec,
   983					     unsigned long liovcnt,
   984					     const struct iovec __user *rvec,
   985					     unsigned long riovcnt,
   986					     unsigned long flags);
   987	asmlinkage long sys_process_vm_writev(pid_t pid,
   988					      const struct iovec __user *lvec,
   989					      unsigned long liovcnt,
   990					      const struct iovec __user *rvec,
   991					      unsigned long riovcnt,
   992					      unsigned long flags);
   993	asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
   994				 unsigned long idx1, unsigned long idx2);
   995	asmlinkage long sys_finit_module(int fd, const char __user *uargs, int flags);
   996	asmlinkage long sys_sched_setattr(pid_t pid,
   997						struct sched_attr __user *attr,
   998						unsigned int flags);
   999	asmlinkage long sys_sched_getattr(pid_t pid,
  1000						struct sched_attr __user *attr,
  1001						unsigned int size,
  1002						unsigned int flags);
  1003	asmlinkage long sys_renameat2(int olddfd, const char __user *oldname,
  1004				      int newdfd, const char __user *newname,
  1005				      unsigned int flags);
  1006	asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
  1007				    void __user *uargs);
  1008	asmlinkage long sys_getrandom(char __user *buf, size_t count,
  1009				      unsigned int flags);
  1010	asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
  1011	asmlinkage long sys_bpf(int cmd, union bpf_attr *attr, unsigned int size);
  1012	asmlinkage long sys_execveat(int dfd, const char __user *filename,
  1013				const char __user *const __user *argv,
  1014				const char __user *const __user *envp, int flags);
  1015	asmlinkage long sys_userfaultfd(int flags);
  1016	asmlinkage long sys_membarrier(int cmd, unsigned int flags, int cpu_id);
  1017	asmlinkage long sys_mlock2(unsigned long start, size_t len, int flags);
  1018	asmlinkage long sys_copy_file_range(int fd_in, loff_t __user *off_in,
  1019					    int fd_out, loff_t __user *off_out,
  1020					    size_t len, unsigned int flags);
  1021	asmlinkage long sys_preadv2(unsigned long fd, const struct iovec __user *vec,
  1022				    unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
  1023				    rwf_t flags);
  1024	asmlinkage long sys_pwritev2(unsigned long fd, const struct iovec __user *vec,
  1025				    unsigned long vlen, unsigned long pos_l, unsigned long pos_h,
  1026				    rwf_t flags);
  1027	asmlinkage long sys_pkey_mprotect(unsigned long start, size_t len,
  1028					  unsigned long prot, int pkey);
  1029	asmlinkage long sys_pkey_alloc(unsigned long flags, unsigned long init_val);
  1030	asmlinkage long sys_pkey_free(int pkey);
  1031	asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
  1032				  unsigned mask, struct statx __user *buffer);
  1033	asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
  1034				 int flags, uint32_t sig);
  1035	asmlinkage long sys_open_tree(int dfd, const char __user *path, unsigned flags);
  1036	asmlinkage long sys_move_mount(int from_dfd, const char __user *from_path,
  1037				       int to_dfd, const char __user *to_path,
  1038				       unsigned int ms_flags);
  1039	asmlinkage long sys_mount_setattr(int dfd, const char __user *path,
  1040					  unsigned int flags,
  1041					  struct mount_attr __user *uattr, size_t usize);
  1042	asmlinkage long sys_fsopen(const char __user *fs_name, unsigned int flags);
  1043	asmlinkage long sys_fsconfig(int fs_fd, unsigned int cmd, const char __user *key,
  1044				     const void __user *value, int aux);
  1045	asmlinkage long sys_fsmount(int fs_fd, unsigned int flags, unsigned int ms_flags);
  1046	asmlinkage long sys_fspick(int dfd, const char __user *path, unsigned int flags);
  1047	asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
  1048					       siginfo_t __user *info,
  1049					       unsigned int flags);
  1050	asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
  1051	asmlinkage long sys_landlock_create_ruleset(const struct landlock_ruleset_attr __user *attr,
  1052			size_t size, __u32 flags);
  1053	asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum landlock_rule_type rule_type,
  1054			const void __user *rule_attr, __u32 flags);
  1055	asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 flags);
  1056	asmlinkage long sys_memfd_secret(unsigned int flags);
  1057	asmlinkage long sys_set_mempolicy_home_node(unsigned long start, unsigned long len,
  1058						    unsigned long home_node,
  1059						    unsigned long flags);
> 1060	asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
  1061
  

Hi Casey,

I love your patch! Perhaps something to improve:

[auto build test WARNING on kees/for-next/hardening]
[also build test WARNING on pcmoore-selinux/next acme/perf/core linus/master v6.1-rc2 next-20221025]
[cannot apply to tip/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
patch link:    https://lore.kernel.org/r/20221025184519.13231-7-casey%40schaufler-ca.com
patch subject: [PATCH v1 6/8] LSM: lsm_self_attr syscall for LSM self attributes
config: s390-allmodconfig (attached as .config)
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/c9d17b230f202246a9451fbdefac8c1720eb68a6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
        git checkout c9d17b230f202246a9451fbdefac8c1720eb68a6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> security/lsm_syscalls.c:51: warning: expecting prototype for lsm_self_attr(). Prototype was for sys_lsm_self_attr() instead


vim +51 security/lsm_syscalls.c

    33	
    34	/**
    35	 * lsm_self_attr - Return current task's security module attributes
    36	 * @ctx: the LSM contexts
    37	 * @size: size of @ctx, updated on return
    38	 * @flags: reserved for future use, must be zero
    39	 *
    40	 * Returns the calling task's LSM contexts. On success this
    41	 * function returns the number of @ctx array elements. This value
    42	 * may be zero if there are no LSM contexts assigned. If @size is
    43	 * insufficient to contain the return data -E2BIG is returned and
    44	 * @size is set to the minimum required size. In all other cases
    45	 * a negative value indicating the error is returned.
    46	 */
    47	SYSCALL_DEFINE3(lsm_self_attr,
    48		       struct lsm_ctx __user *, ctx,
    49		       size_t __user *, size,
    50		       int, flags)
  > 51	{
  

Hi Casey,

I love your patch! Yet something to improve:

[auto build test ERROR on kees/for-next/hardening]
[also build test ERROR on pcmoore-selinux/next acme/perf/core linus/master v6.1-rc2 next-20221026]
[cannot apply to tip/perf/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
patch link:    https://lore.kernel.org/r/20221025184519.13231-7-casey%40schaufler-ca.com
patch subject: [PATCH v1 6/8] LSM: lsm_self_attr syscall for LSM self attributes
config: arm-randconfig-r031-20221025 (attached as .config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/intel-lab-lkp/linux/commit/c9d17b230f202246a9451fbdefac8c1720eb68a6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Casey-Schaufler/LSM-Identify-modules-by-more-than-name/20221026-034541
        git checkout c9d17b230f202246a9451fbdefac8c1720eb68a6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from security/lsm_syscalls.c:15:
   include/linux/syscalls.h:1060:42: warning: declaration of 'struct lsm_ctx' will not be visible outside of this function [-Wvisibility]
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                                            ^
>> security/lsm_syscalls.c:47:1: error: conflicting types for 'sys_lsm_self_attr'
   SYSCALL_DEFINE3(lsm_self_attr,
   ^
   include/linux/syscalls.h:220:36: note: expanded from macro 'SYSCALL_DEFINE3'
   #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                      ^
   include/linux/syscalls.h:229:2: note: expanded from macro 'SYSCALL_DEFINEx'
           __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
           ^
   include/linux/syscalls.h:243:18: note: expanded from macro '__SYSCALL_DEFINEx'
           asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))       \
                           ^
   <scratch space>:65:1: note: expanded from here
   sys_lsm_self_attr
   ^
   include/linux/syscalls.h:1060:17: note: previous declaration is here
   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t *size, int flags);
                   ^
   1 warning and 1 error generated.


vim +/sys_lsm_self_attr +47 security/lsm_syscalls.c

    33	
    34	/**
    35	 * lsm_self_attr - Return current task's security module attributes
    36	 * @ctx: the LSM contexts
    37	 * @size: size of @ctx, updated on return
    38	 * @flags: reserved for future use, must be zero
    39	 *
    40	 * Returns the calling task's LSM contexts. On success this
    41	 * function returns the number of @ctx array elements. This value
    42	 * may be zero if there are no LSM contexts assigned. If @size is
    43	 * insufficient to contain the return data -E2BIG is returned and
    44	 * @size is set to the minimum required size. In all other cases
    45	 * a negative value indicating the error is returned.
    46	 */
  > 47	SYSCALL_DEFINE3(lsm_self_attr,
  

On Tue, Oct 25, 2022 at 2:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Create a system call lsm_self_attr() to provide the security
> module maintained attributes of the current process. Historically
> these attributes have been exposed to user space via entries in
> procfs under /proc/self/attr.
>
> Attributes are provided as a collection of lsm_ctx structures
> which are placed into a user supplied buffer. Each structure
> identifys the security module providing the attribute, which
> of the possible attributes is provided, the size of the
> attribute, and finally the attribute value. The format of the
> attribute value is defined by the security module, but will
> always be \0 terminated. The ctx_len value will be larger than
> strlen(ctx).
>
>         ------------------------------
>         | unsigned int id            |
>         ------------------------------
>         | unsigned int flags         |
>         ------------------------------
>         | __kernel_size_t ctx_len    |
>         ------------------------------
>         | unsigned char ctx[ctx_len] |
>         ------------------------------
>         | unsigned int id            |
>         ------------------------------
>         | unsigned int flags         |
>         ------------------------------
>         | __kernel_size_t ctx_len    |
>         ------------------------------
>         | unsigned char ctx[ctx_len] |
>         ------------------------------
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  include/linux/syscalls.h |   2 +
>  include/uapi/linux/lsm.h |  21 ++++++
>  kernel/sys_ni.c          |   3 +
>  security/Makefile        |   1 +
>  security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
>  5 files changed, 183 insertions(+)
>  create mode 100644 security/lsm_syscalls.c

...

> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> index 61e13b1b9ece..1d27fb5b7746 100644
> --- a/include/uapi/linux/lsm.h
> +++ b/include/uapi/linux/lsm.h
> @@ -9,6 +9,27 @@
>  #ifndef _UAPI_LINUX_LSM_H
>  #define _UAPI_LINUX_LSM_H
>
> +#include <linux/types.h>
> +#include <linux/unistd.h>
> +
> +/**
> + * struct lsm_ctx - LSM context
> + * @id: the LSM id number, see LSM_ID_XXX
> + * @flags: context specifier and LSM specific flags
> + * @ctx_len: the size of @ctx
> + * @ctx: the LSM context, a nul terminated string
> + *
> + * @ctx in a nul terminated string.
> + *     (strlen(@ctx) < @ctx_len) is always true.
> + *     (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
> + */

We can do better than this, or rather we *should* do better than this.
One of the big advantages of creating a new API is we can fix some of
the silly things we have had to do in the past, including the
"sometimes" NUL terminator on strings.  For this LSM syscalls let's
make a promise that all human readable strings will be properly NUL
terminated; currently this includes all LSM contexts, and likely will
remain that way forever for various reasons, but let's leave the door
open for arbitrary blobs (see the "special" LSM ID discussion from
earlier in the patchset).  With that in mind I might suggest the
following:

  /**
   * struct lsm_ctx - LSM context
   * @id: the LSM id number, see LSM_ID_XXX
   * @flags: context specifier and LSM specific flags
   * @ctx_len: the size of @ctx
   * @ctx: the LSM context, see description
   *
   * For LSMs which provide human readable contexts @ctx will be a nul
   * terminated string and @ctx_len will include the size of the string
   * and the nul terminator, e.g. 'ctx_len == strlen(ctx) + 1'.  For LSMs
   * which provide binary-only contexts @cts will be a binary blob with
   * @ctx_len being the exact value of the blob.  The type of the @ctx,
   * human readable string or binary, can be determined by inspecting
   * both @id and @flags.
   *
   * If a given LSM @id does not define a set of values for use in the
   * @flags field, @flags MUST be set to zero.
   */
  struct lsm_ctx {
    __u32 id;
    __U32 flags;
    __kernel_size_t ctx_len;
    __u8 ctx[];
  };

> +struct lsm_ctx {
> +       unsigned int            id;
> +       unsigned int            flags;
> +       __kernel_size_t         ctx_len;
> +       unsigned char           ctx[];
> +};

I agree with Greg, we should be explicit about variable sizing, let's
make sure everything in the UAPI header is defined in terms of
__uXX/__sXX.  This includes strings as __u8 arrays.

Also, I sorta despite the 'let's line all the struct fields up
horizontally!' approach in struct/variable definitions.  I personally
think it looks horrible and it clutters up future patches.  Please
don't do this unless the individual file already does it, and since we
are creating this new please don't :)

> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> new file mode 100644
> index 000000000000..da0fab7065e2
> --- /dev/null
> +++ b/security/lsm_syscalls.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * System calls implementing the Linux Security Module API.
> + *
> + *  Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
> + *  Copyright (C) Intel Corporation
> + */
> +
> +#include <asm/current.h>
> +#include <linux/compiler_types.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/security.h>
> +#include <linux/stddef.h>
> +#include <linux/syscalls.h>
> +#include <linux/types.h>
> +#include <linux/lsm_hooks.h>
> +#include <uapi/linux/lsm.h>
> +
> +struct feature_map {
> +       char *name;
> +       int feature;
> +};
> +
> +static const struct feature_map lsm_attr_names[] = {
> +       { .name = "current",    .feature = LSM_ATTR_CURRENT, },
> +       { .name = "exec",       .feature = LSM_ATTR_EXEC, },
> +       { .name = "fscreate",   .feature = LSM_ATTR_FSCREATE, },
> +       { .name = "keycreate",  .feature = LSM_ATTR_KEYCREATE, },
> +       { .name = "prev",       .feature = LSM_ATTR_PREV, },
> +       { .name = "sockcreate", .feature = LSM_ATTR_SOCKCREATE, },
> +};
> +
> +/**
> + * lsm_self_attr - Return current task's security module attributes
> + * @ctx: the LSM contexts
> + * @size: size of @ctx, updated on return
> + * @flags: reserved for future use, must be zero
> + *
> + * Returns the calling task's LSM contexts. On success this
> + * function returns the number of @ctx array elements. This value
> + * may be zero if there are no LSM contexts assigned. If @size is
> + * insufficient to contain the return data -E2BIG is returned and
> + * @size is set to the minimum required size. In all other cases
> + * a negative value indicating the error is returned.
> + */
> +SYSCALL_DEFINE3(lsm_self_attr,
> +              struct lsm_ctx __user *, ctx,
> +              size_t __user *, size,
> +              int, flags)

See my comments above about UAPI types, let's change this to something
like this:

[NOTE: I'm assuming it is safe to use __XXX types in syscall declarations?]

  SYSCALL_DEFINE3(lsm_self_attr,
                 struct lsm_ctx __user *, ctx,
                 __kernel_size_t __user *, size,
                 __u32, flags)

> +{
> +       struct lsm_ctx *final = NULL;
> +       struct lsm_ctx *interum;
> +       struct lsm_ctx *ip;
> +       void *curr;
> +       char **interum_ctx;
> +       char *cp;
> +       size_t total_size = 0;
> +       int count = 0;
> +       int attr;
> +       int len;
> +       int rc = 0;
> +       int i;

Ungh, reverse christmas trees ... I kinda hate it from a style
perspective, enough to mention it here, but I'm not going to be petty
enough to say "change it".

However, if you did want to flip it upside down (normal christmas
tree?) during the respin I would be grateful ;)

--
paul-moore.com
  

On 11/9/2022 3:34 PM, Paul Moore wrote:
> On Tue, Oct 25, 2022 at 2:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Create a system call lsm_self_attr() to provide the security
>> module maintained attributes of the current process. Historically
>> these attributes have been exposed to user space via entries in
>> procfs under /proc/self/attr.
>>
>> Attributes are provided as a collection of lsm_ctx structures
>> which are placed into a user supplied buffer. Each structure
>> identifys the security module providing the attribute, which
>> of the possible attributes is provided, the size of the
>> attribute, and finally the attribute value. The format of the
>> attribute value is defined by the security module, but will
>> always be \0 terminated. The ctx_len value will be larger than
>> strlen(ctx).
>>
>>         ------------------------------
>>         | unsigned int id            |
>>         ------------------------------
>>         | unsigned int flags         |
>>         ------------------------------
>>         | __kernel_size_t ctx_len    |
>>         ------------------------------
>>         | unsigned char ctx[ctx_len] |
>>         ------------------------------
>>         | unsigned int id            |
>>         ------------------------------
>>         | unsigned int flags         |
>>         ------------------------------
>>         | __kernel_size_t ctx_len    |
>>         ------------------------------
>>         | unsigned char ctx[ctx_len] |
>>         ------------------------------
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  include/linux/syscalls.h |   2 +
>>  include/uapi/linux/lsm.h |  21 ++++++
>>  kernel/sys_ni.c          |   3 +
>>  security/Makefile        |   1 +
>>  security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 183 insertions(+)
>>  create mode 100644 security/lsm_syscalls.c
> ..
>
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> index 61e13b1b9ece..1d27fb5b7746 100644
>> --- a/include/uapi/linux/lsm.h
>> +++ b/include/uapi/linux/lsm.h
>> @@ -9,6 +9,27 @@
>>  #ifndef _UAPI_LINUX_LSM_H
>>  #define _UAPI_LINUX_LSM_H
>>
>> +#include <linux/types.h>
>> +#include <linux/unistd.h>
>> +
>> +/**
>> + * struct lsm_ctx - LSM context
>> + * @id: the LSM id number, see LSM_ID_XXX
>> + * @flags: context specifier and LSM specific flags
>> + * @ctx_len: the size of @ctx
>> + * @ctx: the LSM context, a nul terminated string
>> + *
>> + * @ctx in a nul terminated string.
>> + *     (strlen(@ctx) < @ctx_len) is always true.
>> + *     (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
>> + */
> We can do better than this, or rather we *should* do better than this.
> One of the big advantages of creating a new API is we can fix some of
> the silly things we have had to do in the past, including the
> "sometimes" NUL terminator on strings.  For this LSM syscalls let's
> make a promise that all human readable strings will be properly NUL
> terminated;

It requires effort and buffer management to ensure that ctx_len == strlen(ctx)+1,
but if you think it's important, sure.

>  currently this includes all LSM contexts, and likely will
> remain that way forever for various reasons, but let's leave the door
> open for arbitrary blobs (see the "special" LSM ID discussion from
> earlier in the patchset).  With that in mind I might suggest the
> following:
>
>   /**
>    * struct lsm_ctx - LSM context
>    * @id: the LSM id number, see LSM_ID_XXX
>    * @flags: context specifier and LSM specific flags
>    * @ctx_len: the size of @ctx
>    * @ctx: the LSM context, see description
>    *
>    * For LSMs which provide human readable contexts @ctx will be a nul
>    * terminated string and @ctx_len will include the size of the string
>    * and the nul terminator, e.g. 'ctx_len == strlen(ctx) + 1'.  For LSMs
>    * which provide binary-only contexts @cts will be a binary blob with
>    * @ctx_len being the exact value of the blob.  The type of the @ctx,
>    * human readable string or binary, can be determined by inspecting
>    * both @id and @flags.

I'd go a touch further, defining LSM_ATTR_BINARY as a flag and demanding
that any attribute that isn't a nul terminated string be thus identified,
even if it is always binary. Thus, LSM_ATTR_CURRENT and LSM_ATTR_BINARY
together would be an error.

>    *
>    * If a given LSM @id does not define a set of values for use in the
>    * @flags field, @flags MUST be set to zero.
>    */
>   struct lsm_ctx {
>     __u32 id;
>     __U32 flags;
>     __kernel_size_t ctx_len;
>     __u8 ctx[];
>   };
>
>> +struct lsm_ctx {
>> +       unsigned int            id;
>> +       unsigned int            flags;
>> +       __kernel_size_t         ctx_len;
>> +       unsigned char           ctx[];
>> +};
> I agree with Greg, we should be explicit about variable sizing, let's
> make sure everything in the UAPI header is defined in terms of
> __uXX/__sXX.  This includes strings as __u8 arrays.
>
> Also, I sorta despite the 'let's line all the struct fields up
> horizontally!' approach in struct/variable definitions.

Kids. Got no respect for tradition.

>   I personally
> think it looks horrible and it clutters up future patches.  Please
> don't do this unless the individual file already does it, and since we
> are creating this new please don't :)
>
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> new file mode 100644
>> index 000000000000..da0fab7065e2
>> --- /dev/null
>> +++ b/security/lsm_syscalls.c
>> @@ -0,0 +1,156 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * System calls implementing the Linux Security Module API.
>> + *
>> + *  Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
>> + *  Copyright (C) Intel Corporation
>> + */
>> +
>> +#include <asm/current.h>
>> +#include <linux/compiler_types.h>
>> +#include <linux/err.h>
>> +#include <linux/errno.h>
>> +#include <linux/security.h>
>> +#include <linux/stddef.h>
>> +#include <linux/syscalls.h>
>> +#include <linux/types.h>
>> +#include <linux/lsm_hooks.h>
>> +#include <uapi/linux/lsm.h>
>> +
>> +struct feature_map {
>> +       char *name;
>> +       int feature;
>> +};
>> +
>> +static const struct feature_map lsm_attr_names[] = {
>> +       { .name = "current",    .feature = LSM_ATTR_CURRENT, },
>> +       { .name = "exec",       .feature = LSM_ATTR_EXEC, },
>> +       { .name = "fscreate",   .feature = LSM_ATTR_FSCREATE, },
>> +       { .name = "keycreate",  .feature = LSM_ATTR_KEYCREATE, },
>> +       { .name = "prev",       .feature = LSM_ATTR_PREV, },
>> +       { .name = "sockcreate", .feature = LSM_ATTR_SOCKCREATE, },
>> +};
>> +
>> +/**
>> + * lsm_self_attr - Return current task's security module attributes
>> + * @ctx: the LSM contexts
>> + * @size: size of @ctx, updated on return
>> + * @flags: reserved for future use, must be zero
>> + *
>> + * Returns the calling task's LSM contexts. On success this
>> + * function returns the number of @ctx array elements. This value
>> + * may be zero if there are no LSM contexts assigned. If @size is
>> + * insufficient to contain the return data -E2BIG is returned and
>> + * @size is set to the minimum required size. In all other cases
>> + * a negative value indicating the error is returned.
>> + */
>> +SYSCALL_DEFINE3(lsm_self_attr,
>> +              struct lsm_ctx __user *, ctx,
>> +              size_t __user *, size,
>> +              int, flags)
> See my comments above about UAPI types, let's change this to something
> like this:
>
> [NOTE: I'm assuming it is safe to use __XXX types in syscall declarations?]
>
>   SYSCALL_DEFINE3(lsm_self_attr,
>                  struct lsm_ctx __user *, ctx,
>                  __kernel_size_t __user *, size,
>                  __u32, flags)
>
>> +{
>> +       struct lsm_ctx *final = NULL;
>> +       struct lsm_ctx *interum;
>> +       struct lsm_ctx *ip;
>> +       void *curr;
>> +       char **interum_ctx;
>> +       char *cp;
>> +       size_t total_size = 0;
>> +       int count = 0;
>> +       int attr;
>> +       int len;
>> +       int rc = 0;
>> +       int i;
> Ungh, reverse christmas trees ... I kinda hate it from a style
> perspective, enough to mention it here, but I'm not going to be petty
> enough to say "change it".

I really don't care. Last I saw reverse christmas tree was the officially
recommended style. I don't care one way or the other.

>
> However, if you did want to flip it upside down (normal christmas
> tree?) during the respin I would be grateful ;)
>
> --
> paul-moore.com
  

On Wed, Nov 9, 2022 at 8:32 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 11/9/2022 3:34 PM, Paul Moore wrote:
> > On Tue, Oct 25, 2022 at 2:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> Create a system call lsm_self_attr() to provide the security
> >> module maintained attributes of the current process. Historically
> >> these attributes have been exposed to user space via entries in
> >> procfs under /proc/self/attr.
> >>
> >> Attributes are provided as a collection of lsm_ctx structures
> >> which are placed into a user supplied buffer. Each structure
> >> identifys the security module providing the attribute, which
> >> of the possible attributes is provided, the size of the
> >> attribute, and finally the attribute value. The format of the
> >> attribute value is defined by the security module, but will
> >> always be \0 terminated. The ctx_len value will be larger than
> >> strlen(ctx).
> >>
> >>         ------------------------------
> >>         | unsigned int id            |
> >>         ------------------------------
> >>         | unsigned int flags         |
> >>         ------------------------------
> >>         | __kernel_size_t ctx_len    |
> >>         ------------------------------
> >>         | unsigned char ctx[ctx_len] |
> >>         ------------------------------
> >>         | unsigned int id            |
> >>         ------------------------------
> >>         | unsigned int flags         |
> >>         ------------------------------
> >>         | __kernel_size_t ctx_len    |
> >>         ------------------------------
> >>         | unsigned char ctx[ctx_len] |
> >>         ------------------------------
> >>
> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >> ---
> >>  include/linux/syscalls.h |   2 +
> >>  include/uapi/linux/lsm.h |  21 ++++++
> >>  kernel/sys_ni.c          |   3 +
> >>  security/Makefile        |   1 +
> >>  security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
> >>  5 files changed, 183 insertions(+)
> >>  create mode 100644 security/lsm_syscalls.c
> > ..
> >
> >> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
> >> index 61e13b1b9ece..1d27fb5b7746 100644
> >> --- a/include/uapi/linux/lsm.h
> >> +++ b/include/uapi/linux/lsm.h
> >> @@ -9,6 +9,27 @@
> >>  #ifndef _UAPI_LINUX_LSM_H
> >>  #define _UAPI_LINUX_LSM_H
> >>
> >> +#include <linux/types.h>
> >> +#include <linux/unistd.h>
> >> +
> >> +/**
> >> + * struct lsm_ctx - LSM context
> >> + * @id: the LSM id number, see LSM_ID_XXX
> >> + * @flags: context specifier and LSM specific flags
> >> + * @ctx_len: the size of @ctx
> >> + * @ctx: the LSM context, a nul terminated string
> >> + *
> >> + * @ctx in a nul terminated string.
> >> + *     (strlen(@ctx) < @ctx_len) is always true.
> >> + *     (strlen(@ctx) == @ctx_len + 1) is not guaranteed.
> >> + */
> > We can do better than this, or rather we *should* do better than this.
> > One of the big advantages of creating a new API is we can fix some of
> > the silly things we have had to do in the past, including the
> > "sometimes" NUL terminator on strings.  For this LSM syscalls let's
> > make a promise that all human readable strings will be properly NUL
> > terminated;
>
> It requires effort and buffer management to ensure that ctx_len == strlen(ctx)+1,
> but if you think it's important, sure.

I do, yes.  A safe, familiar, and consistent API is worth a little
extra work.  Ensuring the human readable strings are always nul
terminated is familiar to most everyone who has sat in from of a code
editor, and making sure that the @ctx_len variable indicates the full
length of the @ctx buffer (both for strings and binary blobs) provides
a consistent way to manage the context, even if the application isn't
aware of the exact LSM-specific format.

> >  currently this includes all LSM contexts, and likely will
> > remain that way forever for various reasons, but let's leave the door
> > open for arbitrary blobs (see the "special" LSM ID discussion from
> > earlier in the patchset).  With that in mind I might suggest the
> > following:
> >
> >   /**
> >    * struct lsm_ctx - LSM context
> >    * @id: the LSM id number, see LSM_ID_XXX
> >    * @flags: context specifier and LSM specific flags
> >    * @ctx_len: the size of @ctx
> >    * @ctx: the LSM context, see description
> >    *
> >    * For LSMs which provide human readable contexts @ctx will be a nul
> >    * terminated string and @ctx_len will include the size of the string
> >    * and the nul terminator, e.g. 'ctx_len == strlen(ctx) + 1'.  For LSMs
> >    * which provide binary-only contexts @cts will be a binary blob with
> >    * @ctx_len being the exact value of the blob.  The type of the @ctx,
> >    * human readable string or binary, can be determined by inspecting
> >    * both @id and @flags.
>
> I'd go a touch further, defining LSM_ATTR_BINARY as a flag and demanding
> that any attribute that isn't a nul terminated string be thus identified,
> even if it is always binary. Thus, LSM_ATTR_CURRENT and LSM_ATTR_BINARY
> together would be an error.

No, the class/format of the context (string or binary, and the LSM
specific formatting for each) can be deduced from the LSM ID, @id, and
if necessary the @flags field.  I don't want this API to explicitly
prevent a binary LSM_ATTR_CURRENT if the rest of the system is
modified to support it in the future.

> >    *
> >    * If a given LSM @id does not define a set of values for use in the
> >    * @flags field, @flags MUST be set to zero.
> >    */
> >   struct lsm_ctx {
> >     __u32 id;
> >     __U32 flags;
> >     __kernel_size_t ctx_len;
> >     __u8 ctx[];
> >   };
> >
> >> +struct lsm_ctx {
> >> +       unsigned int            id;
> >> +       unsigned int            flags;
> >> +       __kernel_size_t         ctx_len;
> >> +       unsigned char           ctx[];
> >> +};
> > I agree with Greg, we should be explicit about variable sizing, let's
> > make sure everything in the UAPI header is defined in terms of
> > __uXX/__sXX.  This includes strings as __u8 arrays.
> >
> > Also, I sorta despite the 'let's line all the struct fields up
> > horizontally!' approach in struct/variable definitions.
>
> Kids. Got no respect for tradition.

I think you meant to say, "Kids.          Got no          respect
 for      tradition."

> >   I personally
> > think it looks horrible and it clutters up future patches.  Please
> > don't do this unless the individual file already does it, and since we
> > are creating this new please don't :)
> >
> >> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
> >> new file mode 100644
> >> index 000000000000..da0fab7065e2
> >> --- /dev/null
> >> +++ b/security/lsm_syscalls.c
> >> @@ -0,0 +1,156 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * System calls implementing the Linux Security Module API.
> >> + *
> >> + *  Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
> >> + *  Copyright (C) Intel Corporation
> >> + */
> >> +
> >> +#include <asm/current.h>
> >> +#include <linux/compiler_types.h>
> >> +#include <linux/err.h>
> >> +#include <linux/errno.h>
> >> +#include <linux/security.h>
> >> +#include <linux/stddef.h>
> >> +#include <linux/syscalls.h>
> >> +#include <linux/types.h>
> >> +#include <linux/lsm_hooks.h>
> >> +#include <uapi/linux/lsm.h>
> >> +
> >> +struct feature_map {
> >> +       char *name;
> >> +       int feature;
> >> +};
> >> +
> >> +static const struct feature_map lsm_attr_names[] = {
> >> +       { .name = "current",    .feature = LSM_ATTR_CURRENT, },
> >> +       { .name = "exec",       .feature = LSM_ATTR_EXEC, },
> >> +       { .name = "fscreate",   .feature = LSM_ATTR_FSCREATE, },
> >> +       { .name = "keycreate",  .feature = LSM_ATTR_KEYCREATE, },
> >> +       { .name = "prev",       .feature = LSM_ATTR_PREV, },
> >> +       { .name = "sockcreate", .feature = LSM_ATTR_SOCKCREATE, },
> >> +};
> >> +
> >> +/**
> >> + * lsm_self_attr - Return current task's security module attributes
> >> + * @ctx: the LSM contexts
> >> + * @size: size of @ctx, updated on return
> >> + * @flags: reserved for future use, must be zero
> >> + *
> >> + * Returns the calling task's LSM contexts. On success this
> >> + * function returns the number of @ctx array elements. This value
> >> + * may be zero if there are no LSM contexts assigned. If @size is
> >> + * insufficient to contain the return data -E2BIG is returned and
> >> + * @size is set to the minimum required size. In all other cases
> >> + * a negative value indicating the error is returned.
> >> + */
> >> +SYSCALL_DEFINE3(lsm_self_attr,
> >> +              struct lsm_ctx __user *, ctx,
> >> +              size_t __user *, size,
> >> +              int, flags)
> > See my comments above about UAPI types, let's change this to something
> > like this:
> >
> > [NOTE: I'm assuming it is safe to use __XXX types in syscall declarations?]
> >
> >   SYSCALL_DEFINE3(lsm_self_attr,
> >                  struct lsm_ctx __user *, ctx,
> >                  __kernel_size_t __user *, size,
> >                  __u32, flags)
> >
> >> +{
> >> +       struct lsm_ctx *final = NULL;
> >> +       struct lsm_ctx *interum;
> >> +       struct lsm_ctx *ip;
> >> +       void *curr;
> >> +       char **interum_ctx;
> >> +       char *cp;
> >> +       size_t total_size = 0;
> >> +       int count = 0;
> >> +       int attr;
> >> +       int len;
> >> +       int rc = 0;
> >> +       int i;
> > Ungh, reverse christmas trees ... I kinda hate it from a style
> > perspective, enough to mention it here, but I'm not going to be petty
> > enough to say "change it".
>
> I really don't care. Last I saw reverse christmas tree was the officially
> recommended style. I don't care one way or the other.

I think it is one of those per-subsystem oddities like it or not.

> >
> > However, if you did want to flip it upside down (normal christmas
> > tree?) during the respin I would be grateful ;)
> >
  

On Wed, Nov 9, 2022 at 6:34 PM Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Oct 25, 2022 at 2:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > Create a system call lsm_self_attr() to provide the security
> > module maintained attributes of the current process. Historically
> > these attributes have been exposed to user space via entries in
> > procfs under /proc/self/attr.
> >
> > Attributes are provided as a collection of lsm_ctx structures
> > which are placed into a user supplied buffer. Each structure
> > identifys the security module providing the attribute, which
> > of the possible attributes is provided, the size of the
> > attribute, and finally the attribute value. The format of the
> > attribute value is defined by the security module, but will
> > always be \0 terminated. The ctx_len value will be larger than
> > strlen(ctx).
> >
> >         ------------------------------
> >         | unsigned int id            |
> >         ------------------------------
> >         | unsigned int flags         |
> >         ------------------------------
> >         | __kernel_size_t ctx_len    |
> >         ------------------------------
> >         | unsigned char ctx[ctx_len] |
> >         ------------------------------
> >         | unsigned int id            |
> >         ------------------------------
> >         | unsigned int flags         |
> >         ------------------------------
> >         | __kernel_size_t ctx_len    |
> >         ------------------------------
> >         | unsigned char ctx[ctx_len] |
> >         ------------------------------
> >
> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> > ---
> >  include/linux/syscalls.h |   2 +
> >  include/uapi/linux/lsm.h |  21 ++++++
> >  kernel/sys_ni.c          |   3 +
> >  security/Makefile        |   1 +
> >  security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 183 insertions(+)
> >  create mode 100644 security/lsm_syscalls.c

...

> > +/**
> > + * lsm_self_attr - Return current task's security module attributes
> > + * @ctx: the LSM contexts
> > + * @size: size of @ctx, updated on return
> > + * @flags: reserved for future use, must be zero
> > + *
> > + * Returns the calling task's LSM contexts. On success this
> > + * function returns the number of @ctx array elements. This value
> > + * may be zero if there are no LSM contexts assigned. If @size is
> > + * insufficient to contain the return data -E2BIG is returned and
> > + * @size is set to the minimum required size. In all other cases
> > + * a negative value indicating the error is returned.
> > + */
> > +SYSCALL_DEFINE3(lsm_self_attr,
> > +              struct lsm_ctx __user *, ctx,
> > +              size_t __user *, size,
> > +              int, flags)
>
> See my comments above about UAPI types, let's change this to something
> like this:
>
> [NOTE: I'm assuming it is safe to use __XXX types in syscall declarations?]
>
>   SYSCALL_DEFINE3(lsm_self_attr,
>                  struct lsm_ctx __user *, ctx,
>                  __kernel_size_t __user *, size,
>                  __u32, flags)
>

I wanted to clarify how I originally envisioned this syscall/API, as
it looks like it behaves a bit differently than I originally intended.
My thought was that this syscall would be used to fetch one LSM
attribute context at a time, returning an array of lsm_ctx structs,
with one, and only one, for each LSM that supports that particular
attribute.  If the LSM does not support that attribute, it must not
enter an entry to the array.  Requesting more than one attribute
context per invocation is not allowed.  The idea was to closely
resemble the familiar open("/proc/self/attr/current")/read()/close()
result without relying on procfs and supporting multiple LSMs with an
easy(ish) API.  The new, single syscall should also be faster,
although none of this should be happening in a performance critical
section anyway.

In addition, the lsm_ctx::flags field is intended to convey
information specific to the given LSM, i.e. it should not repeat any
of the flag information specified in the syscall parameters.  I don't
believe any of the currently in-tree LSMs would require any special
flags for their contexts, so this should always be zero/clear in this
initial patchset, but it is something to keep in mind for the future.

Thoughts?

--
paul-moore.com
  

On 11/10/2022 3:36 PM, Paul Moore wrote:
> On Wed, Nov 9, 2022 at 6:34 PM Paul Moore <paul@paul-moore.com> wrote:
>> On Tue, Oct 25, 2022 at 2:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> Create a system call lsm_self_attr() to provide the security
>>> module maintained attributes of the current process. Historically
>>> these attributes have been exposed to user space via entries in
>>> procfs under /proc/self/attr.
>>>
>>> Attributes are provided as a collection of lsm_ctx structures
>>> which are placed into a user supplied buffer. Each structure
>>> identifys the security module providing the attribute, which
>>> of the possible attributes is provided, the size of the
>>> attribute, and finally the attribute value. The format of the
>>> attribute value is defined by the security module, but will
>>> always be \0 terminated. The ctx_len value will be larger than
>>> strlen(ctx).
>>>
>>>         ------------------------------
>>>         | unsigned int id            |
>>>         ------------------------------
>>>         | unsigned int flags         |
>>>         ------------------------------
>>>         | __kernel_size_t ctx_len    |
>>>         ------------------------------
>>>         | unsigned char ctx[ctx_len] |
>>>         ------------------------------
>>>         | unsigned int id            |
>>>         ------------------------------
>>>         | unsigned int flags         |
>>>         ------------------------------
>>>         | __kernel_size_t ctx_len    |
>>>         ------------------------------
>>>         | unsigned char ctx[ctx_len] |
>>>         ------------------------------
>>>
>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>>> ---
>>>  include/linux/syscalls.h |   2 +
>>>  include/uapi/linux/lsm.h |  21 ++++++
>>>  kernel/sys_ni.c          |   3 +
>>>  security/Makefile        |   1 +
>>>  security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
>>>  5 files changed, 183 insertions(+)
>>>  create mode 100644 security/lsm_syscalls.c
> ..
>
>>> +/**
>>> + * lsm_self_attr - Return current task's security module attributes
>>> + * @ctx: the LSM contexts
>>> + * @size: size of @ctx, updated on return
>>> + * @flags: reserved for future use, must be zero
>>> + *
>>> + * Returns the calling task's LSM contexts. On success this
>>> + * function returns the number of @ctx array elements. This value
>>> + * may be zero if there are no LSM contexts assigned. If @size is
>>> + * insufficient to contain the return data -E2BIG is returned and
>>> + * @size is set to the minimum required size. In all other cases
>>> + * a negative value indicating the error is returned.
>>> + */
>>> +SYSCALL_DEFINE3(lsm_self_attr,
>>> +              struct lsm_ctx __user *, ctx,
>>> +              size_t __user *, size,
>>> +              int, flags)
>> See my comments above about UAPI types, let's change this to something
>> like this:
>>
>> [NOTE: I'm assuming it is safe to use __XXX types in syscall declarations?]
>>
>>   SYSCALL_DEFINE3(lsm_self_attr,
>>                  struct lsm_ctx __user *, ctx,
>>                  __kernel_size_t __user *, size,
>>                  __u32, flags)
>>
> I wanted to clarify how I originally envisioned this syscall/API, as
> it looks like it behaves a bit differently than I originally intended.

That's why we're having a review cycle.

> My thought was that this syscall would be used to fetch one LSM
> attribute context at a time, returning an array of lsm_ctx structs,
> with one, and only one, for each LSM that supports that particular
> attribute.

My thought with the interface I proposed is that we don't want a
separate syscall for each attribute: e.g. lsm_get_current(), lsm_get_prev(),
and we don't want a separate syscall for each LSM, e.g. lsm_get_selinux().
We can either specify which attribute is desired or which have been returned.
In either case we need an attribute identifier.

>   If the LSM does not support that attribute, it must not
> enter an entry to the array.  Requesting more than one attribute
> context per invocation is not allowed.

Why? That seems like an unnecessary and inconvenient restriction
for the program that wants to see more than one attribute. A service
that wants to check its fscreate, sockcreate and keycreate to see if
they're appropriate for the container it's running in, for example.

>   The idea was to closely
> resemble the familiar open("/proc/self/attr/current")/read()/close()
> result without relying on procfs and supporting multiple LSMs with an
> easy(ish) API.

	rc = lsm_get_self_attr(struct lsm_ctx *ctx, size, attr_id, flags);

This looks like what you're asking for. It's what I had proposed but with
the attr_id specified in the call rather than returned in the lsm_ctx.

>   The new, single syscall should also be faster,
> although none of this should be happening in a performance critical
> section anyway.

Yes.

> In addition, the lsm_ctx::flags field is intended to convey
> information specific to the given LSM, i.e. it should not repeat any
> of the flag information specified in the syscall parameters.  I don't
> believe any of the currently in-tree LSMs would require any special
> flags for their contexts, so this should always be zero/clear in this
> initial patchset, but it is something to keep in mind for the future.
>
> Thoughts?

I don't have any problem with *what I think* you're suggesting.
To make sure, I'll send a new patch. I'll also address lsm_set_self_attr().
Thank you for the feedback. Let's see if we can converge.

>
> --
> paul-moore.com
  

On Thu, Nov 10, 2022 at 7:36 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 11/10/2022 3:36 PM, Paul Moore wrote:
> > On Wed, Nov 9, 2022 at 6:34 PM Paul Moore <paul@paul-moore.com> wrote:
> >> On Tue, Oct 25, 2022 at 2:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>> Create a system call lsm_self_attr() to provide the security
> >>> module maintained attributes of the current process. Historically
> >>> these attributes have been exposed to user space via entries in
> >>> procfs under /proc/self/attr.
> >>>
> >>> Attributes are provided as a collection of lsm_ctx structures
> >>> which are placed into a user supplied buffer. Each structure
> >>> identifys the security module providing the attribute, which
> >>> of the possible attributes is provided, the size of the
> >>> attribute, and finally the attribute value. The format of the
> >>> attribute value is defined by the security module, but will
> >>> always be \0 terminated. The ctx_len value will be larger than
> >>> strlen(ctx).
> >>>
> >>>         ------------------------------
> >>>         | unsigned int id            |
> >>>         ------------------------------
> >>>         | unsigned int flags         |
> >>>         ------------------------------
> >>>         | __kernel_size_t ctx_len    |
> >>>         ------------------------------
> >>>         | unsigned char ctx[ctx_len] |
> >>>         ------------------------------
> >>>         | unsigned int id            |
> >>>         ------------------------------
> >>>         | unsigned int flags         |
> >>>         ------------------------------
> >>>         | __kernel_size_t ctx_len    |
> >>>         ------------------------------
> >>>         | unsigned char ctx[ctx_len] |
> >>>         ------------------------------
> >>>
> >>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> >>> ---
> >>>  include/linux/syscalls.h |   2 +
> >>>  include/uapi/linux/lsm.h |  21 ++++++
> >>>  kernel/sys_ni.c          |   3 +
> >>>  security/Makefile        |   1 +
> >>>  security/lsm_syscalls.c  | 156 +++++++++++++++++++++++++++++++++++++++
> >>>  5 files changed, 183 insertions(+)
> >>>  create mode 100644 security/lsm_syscalls.c
> > ..
> >
> >>> +/**
> >>> + * lsm_self_attr - Return current task's security module attributes
> >>> + * @ctx: the LSM contexts
> >>> + * @size: size of @ctx, updated on return
> >>> + * @flags: reserved for future use, must be zero
> >>> + *
> >>> + * Returns the calling task's LSM contexts. On success this
> >>> + * function returns the number of @ctx array elements. This value
> >>> + * may be zero if there are no LSM contexts assigned. If @size is
> >>> + * insufficient to contain the return data -E2BIG is returned and
> >>> + * @size is set to the minimum required size. In all other cases
> >>> + * a negative value indicating the error is returned.
> >>> + */
> >>> +SYSCALL_DEFINE3(lsm_self_attr,
> >>> +              struct lsm_ctx __user *, ctx,
> >>> +              size_t __user *, size,
> >>> +              int, flags)
> >> See my comments above about UAPI types, let's change this to something
> >> like this:
> >>
> >> [NOTE: I'm assuming it is safe to use __XXX types in syscall declarations?]
> >>
> >>   SYSCALL_DEFINE3(lsm_self_attr,
> >>                  struct lsm_ctx __user *, ctx,
> >>                  __kernel_size_t __user *, size,
> >>                  __u32, flags)
> >>
> > I wanted to clarify how I originally envisioned this syscall/API, as
> > it looks like it behaves a bit differently than I originally intended.
>
> That's why we're having a review cycle.

 :)

> > My thought was that this syscall would be used to fetch one LSM
> > attribute context at a time, returning an array of lsm_ctx structs,
> > with one, and only one, for each LSM that supports that particular
> > attribute.
>
> My thought with the interface I proposed is that we don't want a
> separate syscall for each attribute: e.g. lsm_get_current(), lsm_get_prev(),
> and we don't want a separate syscall for each LSM, e.g. lsm_get_selinux().

Agreed.  I believe that proposed syscall prototype, leveraging the
@flags parameter, supports this.

> We can either specify which attribute is desired or which have been returned.
> In either case we need an attribute identifier.

I never intended the lsm_ctx::flags field to indicate the attribute
itself, e.g. LSM_ATTR_CURRENT, that is for future use by the
individual LSM which is providing the attribute information in that
lsm_ctx array slot.  With that in mind, there is no way for the
syscall to return *what* attribute is returned, only the attribute
value.  This is intentional as I wanted to avoid using this syscall to
fetch different attributes at the same time (one attribute, with
multiple LSMs providing values is the intent).

The "why" is explained below ...

> >   If the LSM does not support that attribute, it must not
> > enter an entry to the array.  Requesting more than one attribute
> > context per invocation is not allowed.
>
> Why? That seems like an unnecessary and inconvenient restriction
> for the program that wants to see more than one attribute. A service
> that wants to check its fscreate, sockcreate and keycreate to see if
> they're appropriate for the container it's running in, for example.

First off, the currently defined attributes are *very* different in
nature so it is unlikely that a chunk of application code would want
to manipulate more than one in a given function.  That alone is a
fairly weak justification, but the idea that attributes aren't related
is important when one considers the access controls a LSM may place
around the management of these attributes.  Combining multiple
attribute requests in a single syscall could increase confusion as
when one of the requests is blocked but others are allowed.  What do
you do here, do you fail the entire syscall or supply just the
attribute that is allowed?  If you do supply just the attribute that
is allowed, do you return an error code?  How would you be able to
distinguish from a LSM which chose not return an attribute and one
that is actively blocking access to that attribute?

Supporting multiple attributes gets complicated very quickly.  Given
the syscall prototype where we treat the flag parameter as a bitfield,
we do allow ourselves the possibility of supporting this in the
future, but let's keep it simple right now.

It's also important to note that currently applications cannot request
multiple attributes via one action.  Even requesting one single
attribute requires a minimum of three syscalls, so we're definitely
improving things here with this syscall.

> >   The idea was to closely
> > resemble the familiar open("/proc/self/attr/current")/read()/close()
> > result without relying on procfs and supporting multiple LSMs with an
> > easy(ish) API.
>
>         rc = lsm_get_self_attr(struct lsm_ctx *ctx, size, attr_id, flags);
>
> This looks like what you're asking for. It's what I had proposed but with
> the attr_id specified in the call rather than returned in the lsm_ctx.

I was thinking that the attr_id would be conveyed as part of the flags
parameter, but I suppose keeping them separate makes life easier (no
worries about flag collisions with the much more generic attribute
ID).  I would suggest that we still treat the attribute parameter as a
bitfield to allow for the possibility of multiple attributes in one
request; as well as a reordering of the parameters:

 int lsm_get_self_attr(__u32 attr,
                       struct lsm_ctx *ctx,
                       __kernel_size_t *size,
                       __u32 flags);

> > Thoughts?
>
> I don't have any problem with *what I think* you're suggesting.
> To make sure, I'll send a new patch. I'll also address lsm_set_self_attr().
> Thank you for the feedback. Let's see if we can converge.

Sounds good.  Thanks Casey.

--
paul-moore.com