From 83370b31a915493231e5b9addc72e4bef69f8d31 Mon Sep 17 00:00:00 2001 From: Tianyue Ren Date: Fri, 9 Oct 2020 09:36:30 +0800 Subject: selinux: fix error initialization in inode_doinit_with_dentry() Mark the inode security label as invalid if we cannot find a dentry so that we will retry later rather than marking it initialized with the unlabeled SID. Fixes: 9287aed2ad1f ("selinux: Convert isec->lock into a spinlock") Signed-off-by: Tianyue Ren [PM: minor comment tweaks] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 6b1826fc3658..158fc47d8620 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1451,7 +1451,13 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * inode_doinit with a dentry, before these inodes could * be used again by userspace. */ - goto out; + isec->initialized = LABEL_INVALID; + /* + * There is nothing useful to jump to the "out" + * label, except a needless spin lock/unlock + * cycle. + */ + return 0; } rc = inode_doinit_use_xattr(inode, dentry, sbsec->def_sid, @@ -1507,8 +1513,15 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * inode_doinit() with a dentry, before these inodes * could be used again by userspace. */ - if (!dentry) - goto out; + if (!dentry) { + isec->initialized = LABEL_INVALID; + /* + * There is nothing useful to jump to the "out" + * label, except a needless spin lock/unlock + * cycle. + */ + return 0; + } rc = selinux_genfs_get_sid(dentry, sclass, sbsec->flags, &sid); if (rc) { -- cgit v1.2.3 From 44141f58e14317853698f994ca5c3785a0c230d0 Mon Sep 17 00:00:00 2001 From: bauen1 Date: Fri, 9 Oct 2020 14:47:11 +0200 Subject: selinux: allow dontauditx and auditallowx rules to take effect without allowx This allows for dontauditing very specific ioctls e.g. TCGETS without dontauditing every ioctl or granting additional permissions. Now either an allowx, dontauditx or auditallowx rules enables checking for extended permissions. Signed-off-by: Jonathan Hettwer Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'security') diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 9704c8a32303..597b79703584 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -596,9 +596,7 @@ void services_compute_xperms_drivers( node->datum.u.xperms->driver); } - /* If no ioctl commands are allowed, ignore auditallow and auditdeny */ - if (node->key.specified & AVTAB_XPERMS_ALLOWED) - xperms->len = 1; + xperms->len = 1; } /* -- cgit v1.2.3 From 200ea5a2292dc444a818b096ae6a32ba3caa51b9 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 3 Nov 2020 11:49:38 -0500 Subject: selinux: fix inode_doinit_with_dentry() LABEL_INVALID error handling A previous fix, commit 83370b31a915 ("selinux: fix error initialization in inode_doinit_with_dentry()"), changed how failures were handled before a SELinux policy was loaded. Unfortunately that patch was potentially problematic for two reasons: it set the isec->initialized state without holding a lock, and it didn't set the inode's SELinux label to the "default" for the particular filesystem. The later can be a problem if/when a later attempt to revalidate the inode fails and SELinux reverts to the existing inode label. This patch should restore the default inode labeling that existed before the original fix, without affecting the LABEL_INVALID marking such that revalidation will still be attempted in the future. Fixes: 83370b31a915 ("selinux: fix error initialization in inode_doinit_with_dentry()") Reported-by: Sven Schnelle Tested-by: Sven Schnelle Reviewed-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 158fc47d8620..c46312710e73 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -1451,13 +1451,7 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * inode_doinit with a dentry, before these inodes could * be used again by userspace. */ - isec->initialized = LABEL_INVALID; - /* - * There is nothing useful to jump to the "out" - * label, except a needless spin lock/unlock - * cycle. - */ - return 0; + goto out_invalid; } rc = inode_doinit_use_xattr(inode, dentry, sbsec->def_sid, @@ -1513,15 +1507,8 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent * inode_doinit() with a dentry, before these inodes * could be used again by userspace. */ - if (!dentry) { - isec->initialized = LABEL_INVALID; - /* - * There is nothing useful to jump to the "out" - * label, except a needless spin lock/unlock - * cycle. - */ - return 0; - } + if (!dentry) + goto out_invalid; rc = selinux_genfs_get_sid(dentry, sclass, sbsec->flags, &sid); if (rc) { @@ -1546,11 +1533,10 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent out: spin_lock(&isec->lock); if (isec->initialized == LABEL_PENDING) { - if (!sid || rc) { + if (rc) { isec->initialized = LABEL_INVALID; goto out_unlock; } - isec->initialized = LABEL_INITIALIZED; isec->sid = sid; } @@ -1558,6 +1544,15 @@ out: out_unlock: spin_unlock(&isec->lock); return rc; + +out_invalid: + spin_lock(&isec->lock); + if (isec->initialized == LABEL_PENDING) { + isec->initialized = LABEL_INVALID; + isec->sid = sid; + } + spin_unlock(&isec->lock); + return 0; } /* Convert a Linux signal to an access vector. */ -- cgit v1.2.3 From b159e86b5a2ab826b3a292756072f4cc523675ab Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Wed, 4 Nov 2020 13:01:10 +0100 Subject: selinux: drop super_block backpointer from superblock_security_struct It appears to have been needed for selinux_complete_init() in the past, but today it's useless. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 5 ++--- security/selinux/include/objsec.h | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c46312710e73..ca5c5623b46d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -600,7 +600,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, { const struct cred *cred = current_cred(); struct superblock_security_struct *sbsec = sb->s_security; - struct dentry *root = sbsec->sb->s_root; + struct dentry *root = sb->s_root; struct selinux_mnt_opts *opts = mnt_opts; struct inode_security_struct *root_isec; u32 fscontext_sid = 0, context_sid = 0, rootcontext_sid = 0; @@ -1080,7 +1080,7 @@ static int selinux_sb_show_options(struct seq_file *m, struct super_block *sb) return rc; } if (sbsec->flags & ROOTCONTEXT_MNT) { - struct dentry *root = sbsec->sb->s_root; + struct dentry *root = sb->s_root; struct inode_security_struct *isec = backing_inode_security(root); seq_putc(m, ','); seq_puts(m, ROOTCONTEXT_STR); @@ -2568,7 +2568,6 @@ static int selinux_sb_alloc_security(struct super_block *sb) mutex_init(&sbsec->lock); INIT_LIST_HEAD(&sbsec->isec_head); spin_lock_init(&sbsec->isec_lock); - sbsec->sb = sb; sbsec->sid = SECINITSID_UNLABELED; sbsec->def_sid = SECINITSID_FILE; sbsec->mntpoint_sid = SECINITSID_UNLABELED; diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h index 330b7b6d44e0..ca4d7ab6a835 100644 --- a/security/selinux/include/objsec.h +++ b/security/selinux/include/objsec.h @@ -61,7 +61,6 @@ struct file_security_struct { }; struct superblock_security_struct { - struct super_block *sb; /* back pointer to sb object */ u32 sid; /* SID of file system superblock */ u32 def_sid; /* default SID for labeling */ u32 mntpoint_sid; /* SECURITY_FS_USE_MNTPOINT context for files */ -- cgit v1.2.3 From b2d99bcb27225fe420a8923b21861aef2bb43d9b Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Fri, 20 Nov 2020 12:32:26 -0600 Subject: selinux: Fix fall-through warnings for Clang In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning by explicitly adding a break statement instead of letting the code fall through to the next case. Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva Signed-off-by: Paul Moore --- security/selinux/hooks.c | 1 + 1 file changed, 1 insertion(+) (limited to 'security') diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ca5c5623b46d..943c2693cec7 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4036,6 +4036,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents) switch (id) { case LOADING_MODULE: rc = selinux_kernel_module_from_file(NULL); + break; default: break; } -- cgit v1.2.3 From 3df98d79215ace13d1e91ddfc5a67a0f5acbd83f Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Sun, 27 Sep 2020 22:38:26 -0400 Subject: lsm,selinux: pass flowi_common instead of flowi to the LSM hooks As pointed out by Herbert in a recent related patch, the LSM hooks do not have the necessary address family information to use the flowi struct safely. As none of the LSMs currently use any of the protocol specific flowi information, replace the flowi pointers with pointers to the address family independent flowi_common struct. Reported-by: Herbert Xu Acked-by: James Morris Signed-off-by: Paul Moore --- security/security.c | 17 +++++++++-------- security/selinux/hooks.c | 4 ++-- security/selinux/include/xfrm.h | 2 +- security/selinux/xfrm.c | 13 +++++++------ 4 files changed, 19 insertions(+), 17 deletions(-) (limited to 'security') diff --git a/security/security.c b/security/security.c index a28045dc9e7f..c08e0eec8b9e 100644 --- a/security/security.c +++ b/security/security.c @@ -2207,15 +2207,16 @@ void security_sk_clone(const struct sock *sk, struct sock *newsk) } EXPORT_SYMBOL(security_sk_clone); -void security_sk_classify_flow(struct sock *sk, struct flowi *fl) +void security_sk_classify_flow(struct sock *sk, struct flowi_common *flic) { - call_void_hook(sk_getsecid, sk, &fl->flowi_secid); + call_void_hook(sk_getsecid, sk, &flic->flowic_secid); } EXPORT_SYMBOL(security_sk_classify_flow); -void security_req_classify_flow(const struct request_sock *req, struct flowi *fl) +void security_req_classify_flow(const struct request_sock *req, + struct flowi_common *flic) { - call_void_hook(req_classify_flow, req, fl); + call_void_hook(req_classify_flow, req, flic); } EXPORT_SYMBOL(security_req_classify_flow); @@ -2407,7 +2408,7 @@ int security_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir) int security_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp, - const struct flowi *fl) + const struct flowi_common *flic) { struct security_hook_list *hp; int rc = LSM_RET_DEFAULT(xfrm_state_pol_flow_match); @@ -2423,7 +2424,7 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x, */ hlist_for_each_entry(hp, &security_hook_heads.xfrm_state_pol_flow_match, list) { - rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl); + rc = hp->hook.xfrm_state_pol_flow_match(x, xp, flic); break; } return rc; @@ -2434,9 +2435,9 @@ int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid) return call_int_hook(xfrm_decode_session, 0, skb, secid, 1); } -void security_skb_classify_flow(struct sk_buff *skb, struct flowi *fl) +void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic) { - int rc = call_int_hook(xfrm_decode_session, 0, skb, &fl->flowi_secid, + int rc = call_int_hook(xfrm_decode_session, 0, skb, &flic->flowic_secid, 0); BUG_ON(rc); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 943c2693cec7..a515abdf115b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5437,9 +5437,9 @@ static void selinux_secmark_refcount_dec(void) } static void selinux_req_classify_flow(const struct request_sock *req, - struct flowi *fl) + struct flowi_common *flic) { - fl->flowi_secid = req->secid; + flic->flowic_secid = req->secid; } static int selinux_tun_dev_alloc_security(void **security) diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index a0b465316292..0a6f34a7a971 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -26,7 +26,7 @@ int selinux_xfrm_state_delete(struct xfrm_state *x); int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir); int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp, - const struct flowi *fl); + const struct flowi_common *flic); #ifdef CONFIG_SECURITY_NETWORK_XFRM extern atomic_t selinux_xfrm_refcount; diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 7314196185d1..c367d36965d4 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -175,9 +175,10 @@ int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid, u8 dir) */ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp, - const struct flowi *fl) + const struct flowi_common *flic) { u32 state_sid; + u32 flic_sid; if (!xp->security) if (x->security) @@ -196,17 +197,17 @@ int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, return 0; state_sid = x->security->ctx_sid; + flic_sid = flic->flowic_secid; - if (fl->flowi_secid != state_sid) + if (flic_sid != state_sid) return 0; /* We don't need a separate SA Vs. policy polmatch check since the SA * is now of the same label as the flow and a flow Vs. policy polmatch * check had already happened in selinux_xfrm_policy_lookup() above. */ - return (avc_has_perm(&selinux_state, - fl->flowi_secid, state_sid, - SECCLASS_ASSOCIATION, ASSOCIATION__SENDTO, - NULL) ? 0 : 1); + return (avc_has_perm(&selinux_state, flic_sid, state_sid, + SECCLASS_ASSOCIATION, ASSOCIATION__SENDTO, + NULL) ? 0 : 1); } static u32 selinux_xfrm_skb_sid_egress(struct sk_buff *skb) -- cgit v1.2.3