From 3675f052b43ba51b99b85b073c7070e083f3e6fb Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Thu, 4 Jul 2019 20:44:44 +0200 Subject: Smack: Don't ignore other bprm->unsafe flags if LSM_UNSAFE_PTRACE is set There is a logic bug in the current smack_bprm_set_creds(): If LSM_UNSAFE_PTRACE is set, but the ptrace state is deemed to be acceptable (e.g. because the ptracer detached in the meantime), the other ->unsafe flags aren't checked. As far as I can tell, this means that something like the following could work (but I haven't tested it): - task A: create task B with fork() - task B: set NO_NEW_PRIVS - task B: install a seccomp filter that makes open() return 0 under some conditions - task B: replace fd 0 with a malicious library - task A: attach to task B with PTRACE_ATTACH - task B: execve() a file with an SMACK64EXEC extended attribute - task A: while task B is still in the middle of execve(), exit (which destroys the ptrace relationship) Make sure that if any flags other than LSM_UNSAFE_PTRACE are set in bprm->unsafe, we reject the execve(). Cc: stable@vger.kernel.org Fixes: 5663884caab1 ("Smack: unify all ptrace accesses in the smack") Signed-off-by: Jann Horn Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 4c5e5a438f8b..d8b59480a01c 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -937,7 +937,8 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm) if (rc != 0) return rc; - } else if (bprm->unsafe) + } + if (bprm->unsafe & ~LSM_UNSAFE_PTRACE) return -EPERM; bsp->smk_task = isp->smk_task; -- cgit From a1a07f22346144d1e2108f9faa2a41fe67579e85 Mon Sep 17 00:00:00 2001 From: luanshi Date: Fri, 5 Jul 2019 10:35:20 +0800 Subject: smack: fix some kernel-doc notations Fix/add kernel-doc notation and fix typos in security/smack/. Signed-off-by: Liguang Zhang Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index d8b59480a01c..336e855e7ab2 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -307,7 +307,7 @@ static struct smack_known *smk_fetch(const char *name, struct inode *ip, /** * init_inode_smack - initialize an inode security blob - * @isp: the blob to initialize + * @inode: inode to extract the info from * @skp: a pointer to the Smack label entry to use in the blob * */ @@ -509,7 +509,7 @@ static int smack_ptrace_traceme(struct task_struct *ptp) /** * smack_syslog - Smack approval on syslog - * @type: message type + * @typefrom_file: unused * * Returns 0 on success, error code otherwise. */ @@ -765,7 +765,7 @@ static int smack_sb_eat_lsm_opts(char *options, void **mnt_opts) /** * smack_set_mnt_opts - set Smack specific mount options * @sb: the file system superblock - * @opts: Smack mount options + * @mnt_opts: Smack mount options * @kern_flags: mount option from kernel space or user space * @set_kern_flags: where to store converted mount opts * @@ -959,7 +959,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm) * smack_inode_alloc_security - allocate an inode blob * @inode: the inode in need of a blob * - * Returns 0 if it gets a blob, -ENOMEM otherwise + * Returns 0 */ static int smack_inode_alloc_security(struct inode *inode) { @@ -1165,7 +1165,7 @@ static int smack_inode_rename(struct inode *old_inode, * * This is the important Smack hook. * - * Returns 0 if access is permitted, -EACCES otherwise + * Returns 0 if access is permitted, an error code otherwise */ static int smack_inode_permission(struct inode *inode, int mask) { @@ -1223,8 +1223,7 @@ static int smack_inode_setattr(struct dentry *dentry, struct iattr *iattr) /** * smack_inode_getattr - Smack check for getting attributes - * @mnt: vfsmount of the object - * @dentry: the object + * @path: path to extract the info from * * Returns 0 if access is permitted, an error code otherwise */ @@ -1871,14 +1870,13 @@ static int smack_file_receive(struct file *file) /** * smack_file_open - Smack dentry open processing * @file: the object - * @cred: task credential * * Set the security blob in the file structure. * Allow the open only if the task has read access. There are * many read operations (e.g. fstat) that you can do with an * fd even if you have the file open write-only. * - * Returns 0 + * Returns 0 if current has access, error code otherwise */ static int smack_file_open(struct file *file) { @@ -1901,7 +1899,7 @@ static int smack_file_open(struct file *file) /** * smack_cred_alloc_blank - "allocate" blank task-level security credentials - * @new: the new credentials + * @cred: the new credentials * @gfp: the atomicity of any memory allocations * * Prepare a blank set of credentials for modification. This must allocate all @@ -1984,7 +1982,7 @@ static void smack_cred_transfer(struct cred *new, const struct cred *old) /** * smack_cred_getsecid - get the secid corresponding to a creds structure - * @c: the object creds + * @cred: the object creds * @secid: where to put the result * * Sets the secid to contain a u32 version of the smack label. @@ -2141,8 +2139,6 @@ static int smack_task_getioprio(struct task_struct *p) /** * smack_task_setscheduler - Smack check on setting scheduler * @p: the task object - * @policy: unused - * @lp: unused * * Return 0 if read access is permitted */ @@ -2612,8 +2608,9 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address) /** * smk_ipv6_port_check - check Smack port access - * @sock: socket + * @sk: socket * @address: address + * @act: the action being taken * * Create or update the port list entry */ @@ -2783,7 +2780,7 @@ static int smack_socket_post_create(struct socket *sock, int family, * * Cross reference the peer labels for SO_PEERSEC * - * Returns 0 on success, and error code otherwise + * Returns 0 */ static int smack_socket_socketpair(struct socket *socka, struct socket *sockb) @@ -3015,13 +3012,13 @@ static int smack_shm_shmctl(struct kern_ipc_perm *isp, int cmd) * * Returns 0 if current has the requested access, error code otherwise */ -static int smack_shm_shmat(struct kern_ipc_perm *ipc, char __user *shmaddr, +static int smack_shm_shmat(struct kern_ipc_perm *isp, char __user *shmaddr, int shmflg) { int may; may = smack_flags_to_may(shmflg); - return smk_curacc_shm(ipc, may); + return smk_curacc_shm(isp, may); } /** @@ -4763,7 +4760,7 @@ static __init void init_smack_known_list(void) /** * smack_init - initialize the smack system * - * Returns 0 + * Returns 0 on success, -ENOMEM is there's no memory */ static __init int smack_init(void) { -- cgit From 3f4287e7d98a2954f20bf96c567fdffcd2b63eb9 Mon Sep 17 00:00:00 2001 From: Jia-Ju Bai Date: Tue, 23 Jul 2019 18:00:15 +0800 Subject: security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb() In smack_socket_sock_rcv_skb(), there is an if statement on line 3920 to check whether skb is NULL: if (skb && skb->secmark != 0) This check indicates skb can be NULL in some cases. But on lines 3931 and 3932, skb is used: ad.a.u.net->netif = skb->skb_iif; ipv6_skb_to_auditdata(skb, &ad.a, NULL); Thus, possible null-pointer dereferences may occur when skb is NULL. To fix these possible bugs, an if statement is added to check skb. These bugs are found by a static analysis tool STCheck written by us. Signed-off-by: Jia-Ju Bai Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security/smack') diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 336e855e7ab2..516f7ba96f1e 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3923,6 +3923,8 @@ access_check: skp = smack_ipv6host_label(&sadd); if (skp == NULL) skp = smack_net_ambient; + if (skb == NULL) + break; #ifdef CONFIG_AUDIT smk_ad_init_net(&ad, __func__, LSM_AUDIT_DATA_NET, &net); ad.a.u.net->family = family; -- cgit From e5bfad3d7acc5702f32aafeb388362994f4d7bd0 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 21 Aug 2019 22:54:41 -0700 Subject: smack: use GFP_NOFS while holding inode_smack::smk_lock inode_smack::smk_lock is taken during smack_d_instantiate(), which is called during a filesystem transaction when creating a file on ext4. Therefore to avoid a deadlock, all code that takes this lock must use GFP_NOFS, to prevent memory reclaim from waiting for the filesystem transaction to complete. Reported-by: syzbot+0eefc1e06a77d327a056@syzkaller.appspotmail.com Cc: stable@vger.kernel.org Signed-off-by: Eric Biggers Signed-off-by: Casey Schaufler --- security/smack/smack_access.c | 6 +++--- security/smack/smack_lsm.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'security/smack') diff --git a/security/smack/smack_access.c b/security/smack/smack_access.c index f1c93a7be9ec..38ac3da4e791 100644 --- a/security/smack/smack_access.c +++ b/security/smack/smack_access.c @@ -465,7 +465,7 @@ char *smk_parse_smack(const char *string, int len) if (i == 0 || i >= SMK_LONGLABEL) return ERR_PTR(-EINVAL); - smack = kzalloc(i + 1, GFP_KERNEL); + smack = kzalloc(i + 1, GFP_NOFS); if (smack == NULL) return ERR_PTR(-ENOMEM); @@ -500,7 +500,7 @@ int smk_netlbl_mls(int level, char *catset, struct netlbl_lsm_secattr *sap, if ((m & *cp) == 0) continue; rc = netlbl_catmap_setbit(&sap->attr.mls.cat, - cat, GFP_KERNEL); + cat, GFP_NOFS); if (rc < 0) { netlbl_catmap_free(sap->attr.mls.cat); return rc; @@ -536,7 +536,7 @@ struct smack_known *smk_import_entry(const char *string, int len) if (skp != NULL) goto freeout; - skp = kzalloc(sizeof(*skp), GFP_KERNEL); + skp = kzalloc(sizeof(*skp), GFP_NOFS); if (skp == NULL) { skp = ERR_PTR(-ENOMEM); goto freeout; diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 516f7ba96f1e..abeb09c30633 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -288,7 +288,7 @@ static struct smack_known *smk_fetch(const char *name, struct inode *ip, if (!(ip->i_opflags & IOP_XATTR)) return ERR_PTR(-EOPNOTSUPP); - buffer = kzalloc(SMK_LONGLABEL, GFP_KERNEL); + buffer = kzalloc(SMK_LONGLABEL, GFP_NOFS); if (buffer == NULL) return ERR_PTR(-ENOMEM); -- cgit