From 733e4467dd068922d64e8b42530ea9b2784175dd Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:02 +0300 Subject: [PATCH 01/18] LSM: fix documentation for sb_copy_data hook The @type argument of the sb_copy_data hook was removed in the commit "LSM/SELinux: Interfaces to allow FS to control mount options" (e0007529893c). This commit removes the description of the @type argument from the LSM documentation. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index a9b8ff578b6b..a4b1966f5c30 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -127,7 +127,6 @@ * options cleanly (a filesystem may modify the data e.g. with strsep()). * This also allows the original mount data to be stripped of security- * specific options to avoid having to make filesystems aware of them. - * @type the type of filesystem being mounted. * @orig the original mount data copied from userspace. * @copy copied data which will be passed to the security module. * Returns 0 if the copy was successful. From 5f4b97555c2e54bc5555be9d8a14ac07ddf82345 Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:03 +0300 Subject: [PATCH 02/18] LSM: fix documentation for the syslog hook The syslog hook was changed in the commit "capabilities/syslog: open code cap_syslog logic to fix build failure" (12b3052c3ee8). The argument @from_file was removed from the hook. This patch updates the documentation for the syslog hook accordingly. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index a4b1966f5c30..37713f7da14e 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1291,8 +1291,7 @@ * Check permission before accessing the kernel message ring or changing * logging to the console. * See the syslog(2) manual page for an explanation of the @type values. - * @type contains the type of action. - * @from_file indicates the context of action (if it came from /proc). + * @type contains the SYSLOG_ACTION_* constant from * Return 0 if permission is granted. * @settime: * Check permission to change the system time. From 68b3edbd9fd852e7fb5aae0f84f99bf2b19b97cb Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:04 +0300 Subject: [PATCH 03/18] LSM: fix documentation for the socket_post_create hook This patch slightly fixes the documentation for the socket_post_create hook. The documentation states that i_security field is accessible through inode field of socket structure (i.e., 'sock->inode->i_security'). There is no inode field in the socket structure. The i_security field is accessible through SOCK_INODE macro. The patch updates the documentation to reflect this. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 37713f7da14e..85beba1cf03d 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -768,9 +768,9 @@ * socket structure, but rather, the socket security information is stored * in the associated inode. Typically, the inode alloc_security hook will * allocate and and attach security information to - * sock->inode->i_security. This hook may be used to update the - * sock->inode->i_security field with additional information that wasn't - * available when the inode was allocated. + * SOCK_INODE(sock)->i_security. This hook may be used to update the + * SOCK_INODE(sock)->i_security field with additional information that + * wasn't available when the inode was allocated. * @sock contains the newly created socket structure. * @family contains the requested protocol family. * @type contains the requested communications type. From a890e6378201626d1723f4f2b92a017e141c1144 Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:05 +0300 Subject: [PATCH 04/18] LSM: fix documentation for the task_setscheduler hook The task_setscheduler hook was changed in the commit "security: remove unused parameter from security_task_setscheduler()" (b0ae19811375). The arguments @policy, @lp were removed from the hook. This patch updates the documentation accordingly. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 85beba1cf03d..866aa62e900c 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -671,10 +671,8 @@ * Return 0 if permission is granted. * @task_setscheduler: * Check permission before setting scheduling policy and/or parameters of - * process @p based on @policy and @lp. + * process @p. * @p contains the task_struct for process. - * @policy contains the scheduling policy. - * @lp contains the scheduling parameters. * Return 0 if permission is granted. * @task_getscheduler: * Check permission before obtaining scheduling information for process From 2f991d7ae86a81eba7e564a0f054d9e5dbfbfe34 Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:06 +0300 Subject: [PATCH 05/18] LSM: fix documentation for the socket_getpeersec_dgram hook The socket_getpeersec_dgram hook was changed in the commit "[AF_UNIX]: Kernel memory leak fix for af_unix datagram getpeersec patch" (dc49c1f94e34). The arguments @secdata and @seclen were changed to @sock and @secid. This patch updates the documentation accordingly. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 866aa62e900c..69b1209038ec 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -873,13 +873,13 @@ * @socket_getpeersec_dgram: * This hook allows the security module to provide peer socket security * state for udp sockets on a per-packet basis to userspace via - * getsockopt SO_GETPEERSEC. The application must first have indicated - * the IP_PASSSEC option via getsockopt. It can then retrieve the + * getsockopt SO_GETPEERSEC. The application must first have indicated + * the IP_PASSSEC option via getsockopt. It can then retrieve the * security state returned by this hook for a packet via the SCM_SECURITY * ancillary message type. - * @skb is the skbuff for the packet being queried - * @secdata is a pointer to a buffer in which to copy the security data - * @seclen is the maximum length for @secdata + * @sock contains the peer socket. May be NULL. + * @skb is the sk_buff for the packet being queried. May be NULL. + * @secid pointer to store the secid of the packet. * Return 0 on success, error on failure. * @sk_alloc_security: * Allocate and attach a security structure to the sk->sk_security field, From 6b6b6476a32f763843c9a3c91dff4d91faa1267e Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:07 +0300 Subject: [PATCH 06/18] LSM: fix documentation for the path_chmod hook The path_chmod hook was changed in the commit "switch security_path_chmod() to struct path *" (cdcf116d44e7). The argument @mnt was removed from the hook, @dentry was changed to @path. This patch updates the documentation accordingly. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 69b1209038ec..1a4e4dda5235 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -319,10 +319,11 @@ * @new_dentry contains the dentry structure of the new link. * Return 0 if permission is granted. * @path_chmod: - * Check for permission to change DAC's permission of a file or directory. - * @dentry contains the dentry structure. - * @mnt contains the vfsmnt structure. - * @mode contains DAC's mode. + * Check for permission to change a mode of the file @path. The new + * mode is specified in @mode. + * @path contains the path structure of the file to change the mode. + * @mode contains the new DAC's permission, which is a bitmask of + * constants from * Return 0 if permission is granted. * @path_chown: * Check for permission to change owner/group of a file or directory. From 5fdd268f6eb8e84f04ae2458cc640f7c7331dd19 Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:08 +0300 Subject: [PATCH 07/18] LSM: fix documentation for the audit_* hooks This patch updates the documentation for the audit_* hooks to use the same arguments names as in the hook's declarations. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 1a4e4dda5235..2f0990f0ed64 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1345,9 +1345,9 @@ * -EINVAL in case of an invalid rule. * * @audit_rule_known: - * Specifies whether given @rule contains any fields related to + * Specifies whether given @krule contains any fields related to * current LSM. - * @rule contains the audit rule of interest. + * @krule contains the audit rule of interest. * Return 1 in case of relation found, 0 otherwise. * * @audit_rule_match: @@ -1356,13 +1356,13 @@ * @secid contains the security id in question. * @field contains the field which relates to current LSM. * @op contains the operator that will be used for matching. - * @rule points to the audit rule that will be checked against. + * @lrule points to the audit rule that will be checked against. * Return 1 if secid matches the rule, 0 if it does not, -ERRNO on failure. * * @audit_rule_free: * Deallocate the LSM audit rule structure previously allocated by * audit_rule_init. - * @rule contains the allocated rule + * @lsmrule contains the allocated rule * * @inode_invalidate_secctx: * Notify the security module that it must revalidate the security context From ab012bc83615e843f14d6ba2556f52c60ecf121f Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:09 +0300 Subject: [PATCH 08/18] LSM: fix documentation for the msg_queue_* hooks The msg_queue_* hooks were changed in the commit "msg/security: Pass kern_ipc_perm not msg_queue into the msg_queue security hooks" (d8c6e8543294). The type of the argument msq was changed from msq_queue to kern_ipc_perm. This patch updates the documentation for the hooks accordingly. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 2f0990f0ed64..8ef4b3b89daa 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1111,41 +1111,41 @@ * * @msg_queue_alloc_security: * Allocate and attach a security structure to the - * msq->q_perm.security field. The security field is initialized to + * @perm->security field. The security field is initialized to * NULL when the structure is first created. - * @msq contains the message queue structure to be modified. + * @perm contains the IPC permissions of the message queue. * Return 0 if operation was successful and permission is granted. * @msg_queue_free_security: - * Deallocate security structure for this message queue. - * @msq contains the message queue structure to be modified. + * Deallocate security field @perm->security for the message queue. + * @perm contains the IPC permissions of the message queue. * @msg_queue_associate: * Check permission when a message queue is requested through the - * msgget system call. This hook is only called when returning the + * msgget system call. This hook is only called when returning the * message queue identifier for an existing message queue, not when a * new message queue is created. - * @msq contains the message queue to act upon. + * @perm contains the IPC permissions of the message queue. * @msqflg contains the operation control flags. * Return 0 if permission is granted. * @msg_queue_msgctl: * Check permission when a message control operation specified by @cmd - * is to be performed on the message queue @msq. - * The @msq may be NULL, e.g. for IPC_INFO or MSG_INFO. - * @msq contains the message queue to act upon. May be NULL. + * is to be performed on the message queue with permissions @perm. + * The @perm may be NULL, e.g. for IPC_INFO or MSG_INFO. + * @perm contains the IPC permissions of the msg queue. May be NULL. * @cmd contains the operation to be performed. * Return 0 if permission is granted. * @msg_queue_msgsnd: * Check permission before a message, @msg, is enqueued on the message - * queue, @msq. - * @msq contains the message queue to send message to. + * queue with permissions @perm. + * @perm contains the IPC permissions of the message queue. * @msg contains the message to be enqueued. * @msqflg contains operational flags. * Return 0 if permission is granted. * @msg_queue_msgrcv: * Check permission before a message, @msg, is removed from the message - * queue, @msq. The @target task structure contains a pointer to the + * queue. The @target task structure contains a pointer to the * process that will be receiving the message (not equal to the current * process when inline receives are being performed). - * @msq contains the message queue to retrieve message from. + * @perm contains the IPC permissions of the message queue. * @msg contains the message destination. * @target contains the task structure for recipient process. * @type contains the type of message requested. @@ -1637,13 +1637,13 @@ union security_list_options { int (*msg_msg_alloc_security)(struct msg_msg *msg); void (*msg_msg_free_security)(struct msg_msg *msg); - int (*msg_queue_alloc_security)(struct kern_ipc_perm *msq); - void (*msg_queue_free_security)(struct kern_ipc_perm *msq); - int (*msg_queue_associate)(struct kern_ipc_perm *msq, int msqflg); - int (*msg_queue_msgctl)(struct kern_ipc_perm *msq, int cmd); - int (*msg_queue_msgsnd)(struct kern_ipc_perm *msq, struct msg_msg *msg, + int (*msg_queue_alloc_security)(struct kern_ipc_perm *perm); + void (*msg_queue_free_security)(struct kern_ipc_perm *perm); + int (*msg_queue_associate)(struct kern_ipc_perm *perm, int msqflg); + int (*msg_queue_msgctl)(struct kern_ipc_perm *perm, int cmd); + int (*msg_queue_msgsnd)(struct kern_ipc_perm *perm, struct msg_msg *msg, int msqflg); - int (*msg_queue_msgrcv)(struct kern_ipc_perm *msq, struct msg_msg *msg, + int (*msg_queue_msgrcv)(struct kern_ipc_perm *perm, struct msg_msg *msg, struct task_struct *target, long type, int mode); From e9220bc8b79aebfaa8bc4fee2e8785618daeb42a Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:10 +0300 Subject: [PATCH 09/18] LSM: fix documentation for the sem_* hooks The sem_* hooks were changed in the commit "sem/security: Pass kern_ipc_perm not sem_array into the sem security hooks" (aefad9593ec5). The type of the argument sma was changed from sem_array to kern_ipc_perm. This patch updates the documentation for the hooks accordingly. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 8ef4b3b89daa..bd402be091af 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1190,34 +1190,34 @@ * Security hooks for System V Semaphores * * @sem_alloc_security: - * Allocate and attach a security structure to the sma->sem_perm.security - * field. The security field is initialized to NULL when the structure is + * Allocate and attach a security structure to the @perm->security + * field. The security field is initialized to NULL when the structure is * first created. - * @sma contains the semaphore structure + * @perm contains the IPC permissions of the semaphore. * Return 0 if operation was successful and permission is granted. * @sem_free_security: - * deallocate security struct for this semaphore - * @sma contains the semaphore structure. + * Deallocate security structure @perm->security for the semaphore. + * @perm contains the IPC permissions of the semaphore. * @sem_associate: * Check permission when a semaphore is requested through the semget - * system call. This hook is only called when returning the semaphore + * system call. This hook is only called when returning the semaphore * identifier for an existing semaphore, not when a new one must be * created. - * @sma contains the semaphore structure. + * @perm contains the IPC permissions of the semaphore. * @semflg contains the operation control flags. * Return 0 if permission is granted. * @sem_semctl: * Check permission when a semaphore operation specified by @cmd is to be - * performed on the semaphore @sma. The @sma may be NULL, e.g. for + * performed on the semaphore. The @perm may be NULL, e.g. for * IPC_INFO or SEM_INFO. - * @sma contains the semaphore structure. May be NULL. + * @perm contains the IPC permissions of the semaphore. May be NULL. * @cmd contains the operation to be performed. * Return 0 if permission is granted. * @sem_semop: * Check permissions before performing operations on members of the - * semaphore set @sma. If the @alter flag is nonzero, the semaphore set + * semaphore set. If the @alter flag is nonzero, the semaphore set * may be modified. - * @sma contains the semaphore structure. + * @perm contains the IPC permissions of the semaphore. * @sops contains the operations to perform. * @nsops contains the number of operations to perform. * @alter contains the flag indicating whether changes are to be made. @@ -1654,11 +1654,11 @@ union security_list_options { int (*shm_shmat)(struct kern_ipc_perm *shp, char __user *shmaddr, int shmflg); - int (*sem_alloc_security)(struct kern_ipc_perm *sma); - void (*sem_free_security)(struct kern_ipc_perm *sma); - int (*sem_associate)(struct kern_ipc_perm *sma, int semflg); - int (*sem_semctl)(struct kern_ipc_perm *sma, int cmd); - int (*sem_semop)(struct kern_ipc_perm *sma, struct sembuf *sops, + int (*sem_alloc_security)(struct kern_ipc_perm *perm); + void (*sem_free_security)(struct kern_ipc_perm *perm); + int (*sem_associate)(struct kern_ipc_perm *perm, int semflg); + int (*sem_semctl)(struct kern_ipc_perm *perm, int cmd); + int (*sem_semop)(struct kern_ipc_perm *perm, struct sembuf *sops, unsigned nsops, int alter); int (*netlink_send)(struct sock *sk, struct sk_buff *skb); From 9c53cb9d5648e9daacf6a21bcd8bb2919bed3536 Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:11 +0300 Subject: [PATCH 10/18] LSM: fix documentation for the shm_* hooks The shm_* hooks were changed in the commit "shm/security: Pass kern_ipc_perm not shmid_kernel into the shm security hooks" (7191adff2a55). The type of the argument shp was changed from shmid_kernel to kern_ipc_perm. This patch updates the documentation for the hooks accordingly. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index bd402be091af..ca7b58c94ce8 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1155,34 +1155,34 @@ * Security hooks for System V Shared Memory Segments * * @shm_alloc_security: - * Allocate and attach a security structure to the shp->shm_perm.security - * field. The security field is initialized to NULL when the structure is + * Allocate and attach a security structure to the @perm->security + * field. The security field is initialized to NULL when the structure is * first created. - * @shp contains the shared memory structure to be modified. + * @perm contains the IPC permissions of the shared memory structure. * Return 0 if operation was successful and permission is granted. * @shm_free_security: - * Deallocate the security struct for this memory segment. - * @shp contains the shared memory structure to be modified. + * Deallocate the security structure @perm->security for the memory segment. + * @perm contains the IPC permissions of the shared memory structure. * @shm_associate: * Check permission when a shared memory region is requested through the - * shmget system call. This hook is only called when returning the shared + * shmget system call. This hook is only called when returning the shared * memory region identifier for an existing region, not when a new shared * memory region is created. - * @shp contains the shared memory structure to be modified. + * @perm contains the IPC permissions of the shared memory structure. * @shmflg contains the operation control flags. * Return 0 if permission is granted. * @shm_shmctl: * Check permission when a shared memory control operation specified by - * @cmd is to be performed on the shared memory region @shp. - * The @shp may be NULL, e.g. for IPC_INFO or SHM_INFO. - * @shp contains shared memory structure to be modified. + * @cmd is to be performed on the shared memory region with permissions @perm. + * The @perm may be NULL, e.g. for IPC_INFO or SHM_INFO. + * @perm contains the IPC permissions of the shared memory structure. * @cmd contains the operation to be performed. * Return 0 if permission is granted. * @shm_shmat: * Check permissions prior to allowing the shmat system call to attach the - * shared memory segment @shp to the data segment of the calling process. - * The attaching address is specified by @shmaddr. - * @shp contains the shared memory structure to be modified. + * shared memory segment with permissions @perm to the data segment of the + * calling process. The attaching address is specified by @shmaddr. + * @perm contains the IPC permissions of the shared memory structure. * @shmaddr contains the address to attach memory region to. * @shmflg contains the operational flags. * Return 0 if permission is granted. @@ -1647,11 +1647,11 @@ union security_list_options { struct task_struct *target, long type, int mode); - int (*shm_alloc_security)(struct kern_ipc_perm *shp); - void (*shm_free_security)(struct kern_ipc_perm *shp); - int (*shm_associate)(struct kern_ipc_perm *shp, int shmflg); - int (*shm_shmctl)(struct kern_ipc_perm *shp, int cmd); - int (*shm_shmat)(struct kern_ipc_perm *shp, char __user *shmaddr, + int (*shm_alloc_security)(struct kern_ipc_perm *perm); + void (*shm_free_security)(struct kern_ipc_perm *perm); + int (*shm_associate)(struct kern_ipc_perm *perm, int shmflg); + int (*shm_shmctl)(struct kern_ipc_perm *perm, int cmd); + int (*shm_shmat)(struct kern_ipc_perm *perm, char __user *shmaddr, int shmflg); int (*sem_alloc_security)(struct kern_ipc_perm *perm); From 8d93e952fba216cd0811247f6360d97e0465d5fc Mon Sep 17 00:00:00 2001 From: Denis Efremov Date: Tue, 26 Feb 2019 23:49:12 +0300 Subject: [PATCH 11/18] LSM: lsm_hooks.h: fix documentation format Fix for name mismatch and omitted colons in the security_list_options documentation. Signed-off-by: Denis Efremov Acked-by: Kees Cook Acked-by: Casey Schaufler Signed-off-by: James Morris --- include/linux/lsm_hooks.h | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index ca7b58c94ce8..a240a3fc5fc4 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -502,7 +502,7 @@ * Return 0 if permission is granted. * @file_lock: * Check permission before performing file locking operations. - * Note: this hook mediates both flock and fcntl style locks. + * Note the hook mediates both flock and fcntl style locks. * @file contains the file structure. * @cmd contains the posix-translated lock operation to perform * (e.g. F_RDLCK, F_WRLCK). @@ -645,12 +645,12 @@ * @p contains the task_struct of process. * @nice contains the new nice value. * Return 0 if permission is granted. - * @task_setioprio + * @task_setioprio: * Check permission before setting the ioprio value of @p to @ioprio. * @p contains the task_struct of process. * @ioprio contains the new ioprio value * Return 0 if permission is granted. - * @task_getioprio + * @task_getioprio: * Check permission before getting the ioprio value of @p. * @p contains the task_struct of process. * Return 0 if permission is granted. @@ -680,7 +680,7 @@ * @p. * @p contains the task_struct for process. * Return 0 if permission is granted. - * @task_movememory + * @task_movememory: * Check permission before moving memory owned by process @p. * @p contains the task_struct for process. * Return 0 if permission is granted. @@ -904,9 +904,9 @@ * @secmark_relabel_packet: * check if the process should be allowed to relabel packets to * the given secid - * @security_secmark_refcount_inc + * @secmark_refcount_inc: * tells the LSM to increment the number of secmark labeling rules loaded - * @security_secmark_refcount_dec + * @secmark_refcount_dec: * tells the LSM to decrement the number of secmark labeling rules loaded * @req_classify_flow: * Sets the flow's sid to the openreq sid. @@ -1294,8 +1294,8 @@ * Return 0 if permission is granted. * @settime: * Check permission to change the system time. - * struct timespec64 is defined in include/linux/time64.h and timezone - * is defined in include/linux/time.h + * struct timespec64 is defined in and timezone + * is defined in * @ts contains new time * @tz contains new timezone * Return 0 if permission is granted. @@ -1337,7 +1337,7 @@ * @audit_rule_init: * Allocate and initialize an LSM audit rule structure. * @field contains the required Audit action. - * Fields flags are defined in include/linux/audit.h + * Fields flags are defined in * @op contains the operator the rule uses. * @rulestr contains the context where the rule will be applied to. * @lsmrule contains a pointer to receive the result. @@ -1375,9 +1375,7 @@ * this hook to initialize the security context in its incore inode to the * value provided by the server for the file when the server returned the * file's attributes to the client. - * * Must be called with inode->i_mutex locked. - * * @inode we wish to set the security context of. * @ctx contains the string which we wish to set in the inode. * @ctxlen contains the length of @ctx. @@ -1390,9 +1388,7 @@ * this hook to change the security context in its incore inode and on the * backing filesystem to a value provided by the client on a SETATTR * operation. - * * Must be called with inode->i_mutex locked. - * * @dentry contains the inode we wish to set the security context of. * @ctx contains the string which we wish to set in the inode. * @ctxlen contains the length of @ctx. @@ -1400,7 +1396,6 @@ * @inode_getsecctx: * On success, returns 0 and fills out @ctx and @ctxlen with the security * context for the given @inode. - * * @inode we wish to get the security context of. * @ctx is a pointer in which to place the allocated security context. * @ctxlen points to the place to put the length of @ctx. From 1b26fcdb748eb20a73f72900d7f5ab537b2809be Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Wed, 27 Mar 2019 00:08:41 +0100 Subject: [PATCH 12/18] Yama: mark local symbols as static sparse complains that Yama defines functions and a variable as non-static even though they don't exist in any header. Fix it by making them static. Signed-off-by: Jann Horn Reviewed-by: Mukesh Ojha Signed-off-by: James Morris --- security/yama/yama_lsm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index 57cc60722dd3..06b14a57b0a4 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -206,7 +206,7 @@ static void yama_ptracer_del(struct task_struct *tracer, * yama_task_free - check for task_pid to remove from exception list * @task: task being removed */ -void yama_task_free(struct task_struct *task) +static void yama_task_free(struct task_struct *task) { yama_ptracer_del(task, task); } @@ -401,7 +401,7 @@ static int yama_ptrace_access_check(struct task_struct *child, * * Returns 0 if following the ptrace is allowed, -ve on error. */ -int yama_ptrace_traceme(struct task_struct *parent) +static int yama_ptrace_traceme(struct task_struct *parent) { int rc = 0; @@ -452,7 +452,7 @@ static int yama_dointvec_minmax(struct ctl_table *table, int write, static int zero; static int max_scope = YAMA_SCOPE_NO_ATTACH; -struct ctl_path yama_sysctl_path[] = { +static struct ctl_path yama_sysctl_path[] = { { .procname = "kernel", }, { .procname = "yama", }, { } From 5c7e372caa35d303e414caeb64ee2243fd3cac3d Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Wed, 27 Mar 2019 16:39:38 +0100 Subject: [PATCH 13/18] security: don't use RCU accessors for cred->session_keyring sparse complains that a bunch of places in kernel/cred.c access cred->session_keyring without the RCU helpers required by the __rcu annotation. cred->session_keyring is written in the following places: - prepare_kernel_cred() [in a new cred struct] - keyctl_session_to_parent() [in a new cred struct] - prepare_creds [in a new cred struct, via memcpy] - install_session_keyring_to_cred() - from install_session_keyring() on new creds - from join_session_keyring() on new creds [twice] - from umh_keys_init() - from call_usermodehelper_exec_async() on new creds All of these writes are before the creds are committed; therefore, cred->session_keyring doesn't need RCU protection. Remove the __rcu annotation and fix up all existing users that use __rcu. Signed-off-by: Jann Horn Signed-off-by: James Morris --- include/linux/cred.h | 2 +- security/keys/process_keys.c | 12 ++++-------- security/keys/request_key.c | 9 ++------- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/include/linux/cred.h b/include/linux/cred.h index ddd45bb74887..efb6edf32de7 100644 --- a/include/linux/cred.h +++ b/include/linux/cred.h @@ -138,7 +138,7 @@ struct cred { #ifdef CONFIG_KEYS unsigned char jit_keyring; /* default keyring to attach requested * keys to */ - struct key __rcu *session_keyring; /* keyring inherited over fork */ + struct key *session_keyring; /* keyring inherited over fork */ struct key *process_keyring; /* keyring private to this process */ struct key *thread_keyring; /* keyring private to this thread */ struct key *request_key_auth; /* assumed request_key authority */ diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index 9320424c4a46..bd7243cb4c85 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -227,6 +227,7 @@ static int install_process_keyring(void) * Install the given keyring as the session keyring of the given credentials * struct, replacing the existing one if any. If the given keyring is NULL, * then install a new anonymous session keyring. + * @cred can not be in use by any task yet. * * Return: 0 on success; -errno on failure. */ @@ -254,7 +255,7 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring) /* install the keyring */ old = cred->session_keyring; - rcu_assign_pointer(cred->session_keyring, keyring); + cred->session_keyring = keyring; if (old) key_put(old); @@ -392,11 +393,8 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) /* search the session keyring */ if (ctx->cred->session_keyring) { - rcu_read_lock(); key_ref = keyring_search_aux( - make_key_ref(rcu_dereference(ctx->cred->session_keyring), 1), - ctx); - rcu_read_unlock(); + make_key_ref(ctx->cred->session_keyring, 1), ctx); if (!IS_ERR(key_ref)) goto found; @@ -612,10 +610,8 @@ try_again: goto reget_creds; } - rcu_read_lock(); - key = rcu_dereference(ctx.cred->session_keyring); + key = ctx.cred->session_keyring; __key_get(key); - rcu_read_unlock(); key_ref = make_key_ref(key, 1); break; diff --git a/security/keys/request_key.c b/security/keys/request_key.c index 2f17d84d46f1..db72dc4d7639 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -142,12 +142,10 @@ static int call_sbin_request_key(struct key *authkey, void *aux) prkey = cred->process_keyring->serial; sprintf(keyring_str[1], "%d", prkey); - rcu_read_lock(); - session = rcu_dereference(cred->session_keyring); + session = cred->session_keyring; if (!session) session = cred->user->session_keyring; sskey = session->serial; - rcu_read_unlock(); sprintf(keyring_str[2], "%d", sskey); @@ -287,10 +285,7 @@ static int construct_get_dest_keyring(struct key **_dest_keyring) /* fall through */ case KEY_REQKEY_DEFL_SESSION_KEYRING: - rcu_read_lock(); - dest_keyring = key_get( - rcu_dereference(cred->session_keyring)); - rcu_read_unlock(); + dest_keyring = key_get(cred->session_keyring); if (dest_keyring) break; From 0b9dc6c9f01c4a726558b82a3b6082a89d264eb5 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Wed, 27 Mar 2019 16:55:08 +0100 Subject: [PATCH 14/18] keys: safe concurrent user->{session,uid}_keyring access The current code can perform concurrent updates and reads on user->session_keyring and user->uid_keyring. Add a comment to struct user_struct to document the nontrivial locking semantics, and use READ_ONCE() for unlocked readers and smp_store_release() for writers to prevent memory ordering issues. Fixes: 69664cf16af4 ("keys: don't generate user and user session keyrings unless they're accessed") Signed-off-by: Jann Horn Signed-off-by: James Morris --- include/linux/sched/user.h | 7 +++++++ security/keys/process_keys.c | 31 +++++++++++++++++-------------- security/keys/request_key.c | 5 +++-- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/include/linux/sched/user.h b/include/linux/sched/user.h index c7b5f86b91a1..468d2565a9fe 100644 --- a/include/linux/sched/user.h +++ b/include/linux/sched/user.h @@ -31,6 +31,13 @@ struct user_struct { atomic_long_t pipe_bufs; /* how many pages are allocated in pipe buffers */ #ifdef CONFIG_KEYS + /* + * These pointers can only change from NULL to a non-NULL value once. + * Writes are protected by key_user_keyring_mutex. + * Unlocked readers should use READ_ONCE() unless they know that + * install_user_keyrings() has been called successfully (which sets + * these members to non-NULL values, preventing further modifications). + */ struct key *uid_keyring; /* UID specific keyring */ struct key *session_keyring; /* UID's default session keyring */ #endif diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c index bd7243cb4c85..f05f7125a7d5 100644 --- a/security/keys/process_keys.c +++ b/security/keys/process_keys.c @@ -58,7 +58,7 @@ int install_user_keyrings(void) kenter("%p{%u}", user, uid); - if (user->uid_keyring && user->session_keyring) { + if (READ_ONCE(user->uid_keyring) && READ_ONCE(user->session_keyring)) { kleave(" = 0 [exist]"); return 0; } @@ -111,8 +111,10 @@ int install_user_keyrings(void) } /* install the keyrings */ - user->uid_keyring = uid_keyring; - user->session_keyring = session_keyring; + /* paired with READ_ONCE() */ + smp_store_release(&user->uid_keyring, uid_keyring); + /* paired with READ_ONCE() */ + smp_store_release(&user->session_keyring, session_keyring); } mutex_unlock(&key_user_keyring_mutex); @@ -340,6 +342,7 @@ void key_fsgid_changed(struct task_struct *tsk) key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) { key_ref_t key_ref, ret, err; + const struct cred *cred = ctx->cred; /* we want to return -EAGAIN or -ENOKEY if any of the keyrings were * searchable, but we failed to find a key or we found a negative key; @@ -353,9 +356,9 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) err = ERR_PTR(-EAGAIN); /* search the thread keyring first */ - if (ctx->cred->thread_keyring) { + if (cred->thread_keyring) { key_ref = keyring_search_aux( - make_key_ref(ctx->cred->thread_keyring, 1), ctx); + make_key_ref(cred->thread_keyring, 1), ctx); if (!IS_ERR(key_ref)) goto found; @@ -371,9 +374,9 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) } /* search the process keyring second */ - if (ctx->cred->process_keyring) { + if (cred->process_keyring) { key_ref = keyring_search_aux( - make_key_ref(ctx->cred->process_keyring, 1), ctx); + make_key_ref(cred->process_keyring, 1), ctx); if (!IS_ERR(key_ref)) goto found; @@ -392,9 +395,9 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) } /* search the session keyring */ - if (ctx->cred->session_keyring) { + if (cred->session_keyring) { key_ref = keyring_search_aux( - make_key_ref(ctx->cred->session_keyring, 1), ctx); + make_key_ref(cred->session_keyring, 1), ctx); if (!IS_ERR(key_ref)) goto found; @@ -413,9 +416,9 @@ key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx) } } /* or search the user-session keyring */ - else if (ctx->cred->user->session_keyring) { + else if (READ_ONCE(cred->user->session_keyring)) { key_ref = keyring_search_aux( - make_key_ref(ctx->cred->user->session_keyring, 1), + make_key_ref(READ_ONCE(cred->user->session_keyring), 1), ctx); if (!IS_ERR(key_ref)) goto found; @@ -602,7 +605,7 @@ try_again: goto error; goto reget_creds; } else if (ctx.cred->session_keyring == - ctx.cred->user->session_keyring && + READ_ONCE(ctx.cred->user->session_keyring) && lflags & KEY_LOOKUP_CREATE) { ret = join_session_keyring(NULL); if (ret < 0) @@ -616,7 +619,7 @@ try_again: break; case KEY_SPEC_USER_KEYRING: - if (!ctx.cred->user->uid_keyring) { + if (!READ_ONCE(ctx.cred->user->uid_keyring)) { ret = install_user_keyrings(); if (ret < 0) goto error; @@ -628,7 +631,7 @@ try_again: break; case KEY_SPEC_USER_SESSION_KEYRING: - if (!ctx.cred->user->session_keyring) { + if (!READ_ONCE(ctx.cred->user->session_keyring)) { ret = install_user_keyrings(); if (ret < 0) goto error; diff --git a/security/keys/request_key.c b/security/keys/request_key.c index db72dc4d7639..75d87f9e0f49 100644 --- a/security/keys/request_key.c +++ b/security/keys/request_key.c @@ -293,11 +293,12 @@ static int construct_get_dest_keyring(struct key **_dest_keyring) /* fall through */ case KEY_REQKEY_DEFL_USER_SESSION_KEYRING: dest_keyring = - key_get(cred->user->session_keyring); + key_get(READ_ONCE(cred->user->session_keyring)); break; case KEY_REQKEY_DEFL_USER_KEYRING: - dest_keyring = key_get(cred->user->uid_keyring); + dest_keyring = + key_get(READ_ONCE(cred->user->uid_keyring)); break; case KEY_REQKEY_DEFL_GROUP_KEYRING: From d1a0846006e4325cc951ca0b05c02ed1d0865006 Mon Sep 17 00:00:00 2001 From: Kangjie Lu Date: Fri, 15 Mar 2019 16:00:25 -0500 Subject: [PATCH 15/18] security: inode: fix a missing check for securityfs_create_file securityfs_create_file may fail. The fix checks its status and returns the error code upstream if it fails. Signed-off-by: Kangjie Lu Signed-off-by: James Morris --- security/inode.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/security/inode.c b/security/inode.c index b7772a9b315e..667f8b15027d 100644 --- a/security/inode.c +++ b/security/inode.c @@ -339,6 +339,11 @@ static int __init securityfs_init(void) #ifdef CONFIG_SECURITY lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL, &lsm_ops); + if (IS_ERR(lsm_dentry)) { + unregister_filesystem(&fs_type); + sysfs_remove_mount_point(kernel_kobj, "security"); + return PTR_ERR(lsm_dentry); + } #endif return 0; } From ecb8e74dac1aaeea4005c9d8f21337f9423f3d8b Mon Sep 17 00:00:00 2001 From: Mukesh Ojha Date: Wed, 27 Mar 2019 13:20:18 +0530 Subject: [PATCH 16/18] Yama: mark function as static Sparse complains yama_task_prctl can be static. Fix it by making it static. Signed-off-by: Mukesh Ojha Signed-off-by: James Morris --- security/yama/yama_lsm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c index 06b14a57b0a4..efac68556b45 100644 --- a/security/yama/yama_lsm.c +++ b/security/yama/yama_lsm.c @@ -222,7 +222,7 @@ static void yama_task_free(struct task_struct *task) * Return 0 on success, -ve on error. -ENOSYS is returned when Yama * does not handle the given option. */ -int yama_task_prctl(int option, unsigned long arg2, unsigned long arg3, +static int yama_task_prctl(int option, unsigned long arg2, unsigned long arg3, unsigned long arg4, unsigned long arg5) { int rc = -ENOSYS; From fe9fd2ef383c2f5883fcd3f7adce0de9ce2556ff Mon Sep 17 00:00:00 2001 From: James Morris Date: Wed, 10 Apr 2019 14:59:20 -0700 Subject: [PATCH 17/18] Revert "security: inode: fix a missing check for securityfs_create_file" This reverts commit d1a0846006e4325cc951ca0b05c02ed1d0865006. From Al Viro: "Rather bad way to do it - generally, register_filesystem() should be the last thing done by initialization. Any modular code that does unregister_filesystem() on failure exit is flat-out broken; here it's not instantly FUBAR, but it's a bloody bad example. What's more, why not let simple_fill_super() do it? Just static int fill_super(struct super_block *sb, void *data, int silent) { static const struct tree_descr files[] = { {"lsm", &lsm_ops, 0444}, {""} }; and to hell with that call of securityfs_create_file() and all its failure handling..." Signed-off-by: James Morris --- security/inode.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/security/inode.c b/security/inode.c index 667f8b15027d..b7772a9b315e 100644 --- a/security/inode.c +++ b/security/inode.c @@ -339,11 +339,6 @@ static int __init securityfs_init(void) #ifdef CONFIG_SECURITY lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL, &lsm_ops); - if (IS_ERR(lsm_dentry)) { - unregister_filesystem(&fs_type); - sysfs_remove_mount_point(kernel_kobj, "security"); - return PTR_ERR(lsm_dentry); - } #endif return 0; } From 6beff00b79ca0b5caf0ce6fb8e11f57311bd95f8 Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Wed, 6 Mar 2019 13:14:12 -0700 Subject: [PATCH 18/18] seccomp: fix up grammar in comment This sentence is kind of a train wreck anyway, but at least dropping the extra pronoun helps somewhat. Signed-off-by: Tycho Andersen Acked-by: Kees Cook Signed-off-by: James Morris --- kernel/seccomp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/seccomp.c b/kernel/seccomp.c index 54a0347ca812..503d02896c5d 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -331,7 +331,7 @@ static int is_ancestor(struct seccomp_filter *parent, * Expects sighand and cred_guard_mutex locks to be held. * * Returns 0 on success, -ve on error, or the pid of a thread which was - * either not in the correct seccomp mode or it did not have an ancestral + * either not in the correct seccomp mode or did not have an ancestral * seccomp filter. */ static inline pid_t seccomp_can_sync_threads(void)