From 0266c25e7c2821181b610595df42cbca6bc93cb8 Mon Sep 17 00:00:00 2001 From: "GONG, Ruiqi" Date: Tue, 25 Jan 2022 15:11:33 +0800 Subject: selinux: access superblock_security_struct in LSM blob way LSM blob has been involved for superblock's security struct. So fix the remaining direct access to sb->s_security by using the LSM blob mechanism. Fixes: 08abe46b2cfc ("selinux: fall back to SECURITY_FS_USE_GENFS if no xattr support") Fixes: 69c4a42d72eb ("lsm,selinux: add new hook to compare new mount to an existing mount") Signed-off-by: GONG, Ruiqi Reviewed-by: Casey Schaufler Signed-off-by: Paul Moore --- security/selinux/hooks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5b6895e4fc29..a0243bae8423 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -479,7 +479,7 @@ static int selinux_is_sblabel_mnt(struct super_block *sb) static int sb_check_xattr_support(struct super_block *sb) { - struct superblock_security_struct *sbsec = sb->s_security; + struct superblock_security_struct *sbsec = selinux_superblock(sb); struct dentry *root = sb->s_root; struct inode *root_inode = d_backing_inode(root); u32 sid; @@ -2647,7 +2647,7 @@ free_opt: static int selinux_sb_mnt_opts_compat(struct super_block *sb, void *mnt_opts) { struct selinux_mnt_opts *opts = mnt_opts; - struct superblock_security_struct *sbsec = sb->s_security; + struct superblock_security_struct *sbsec = selinux_superblock(sb); u32 sid; int rc; -- cgit v1.2.3 From bcb62828e3e8c813b6613db6eb7fd9657db248fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:20 +0100 Subject: selinux: check return value of sel_make_avc_files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sel_make_avc_files() might fail and return a negative errno value on memory allocation failures. Re-add the check of the return value, dropped in 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table"). Reported by clang-analyzer: security/selinux/selinuxfs.c:2129:2: warning: Value stored to 'ret' is never read [deadcode.DeadStores] ret = sel_make_avc_files(dentry); ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~ Fixes: 66f8e2f03c02 ("selinux: sidtab reverse lookup hash table") Signed-off-by: Christian Göttsche Reviewed-by: Nick Desaulniers [PM: description line wrapping, added proper commit ref] Signed-off-by: Paul Moore --- security/selinux/selinuxfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index e4cd7cb856f3..f2f6203e0fff 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -2127,6 +2127,8 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc) } ret = sel_make_avc_files(dentry); + if (ret) + goto err; dentry = sel_make_dir(sb->s_root, "ss", &fsi->last_ino); if (IS_ERR(dentry)) { -- cgit v1.2.3 From 08df49054f311ca04954cf24d1216d3b5ddfd0a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:12 +0100 Subject: selinux: declare path parameters of _genfs_sid const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The path parameter is only read from in security_genfs_sid(), selinux_policy_genfs_sid() and __security_genfs_sid(). Since a string literal is passed as argument, declare the parameter const. Also align the parameter names in the declaration and definition. Reported by clang [-Wwrite-strings]: security/selinux/hooks.c:553:60: error: passing 'const char [2]' to parameter of type 'char *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers] rc = security_genfs_sid(&selinux_state, ... , /, ^~~ ./security/selinux/include/security.h:389:36: note: passing argument to parameter 'name' here const char *fstype, char *name, u16 sclass, ^ Signed-off-by: Christian Göttsche [PM: wrapped description] Signed-off-by: Paul Moore --- security/selinux/include/security.h | 4 ++-- security/selinux/ss/services.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index ac0ece01305a..6482e0efb368 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -386,11 +386,11 @@ int security_get_allow_unknown(struct selinux_state *state); int security_fs_use(struct selinux_state *state, struct super_block *sb); int security_genfs_sid(struct selinux_state *state, - const char *fstype, char *name, u16 sclass, + const char *fstype, const char *path, u16 sclass, u32 *sid); int selinux_policy_genfs_sid(struct selinux_policy *policy, - const char *fstype, char *name, u16 sclass, + const char *fstype, const char *path, u16 sclass, u32 *sid); #ifdef CONFIG_NETLABEL diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 8e92af7dd284..5a7df45bdab1 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2875,7 +2875,7 @@ out_unlock: */ static inline int __security_genfs_sid(struct selinux_policy *policy, const char *fstype, - char *path, + const char *path, u16 orig_sclass, u32 *sid) { @@ -2928,7 +2928,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy, */ int security_genfs_sid(struct selinux_state *state, const char *fstype, - char *path, + const char *path, u16 orig_sclass, u32 *sid) { @@ -2952,7 +2952,7 @@ int security_genfs_sid(struct selinux_state *state, int selinux_policy_genfs_sid(struct selinux_policy *policy, const char *fstype, - char *path, + const char *path, u16 orig_sclass, u32 *sid) { -- cgit v1.2.3 From d3b1161f29cf479b86d4c3c6f200a8eb27254877 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:13 +0100 Subject: selinux: declare name parameter of hash_eval const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit String literals are passed as second argument to hash_eval(). Also the parameter is already declared const in the DEBUG_HASHES configuration. Reported by clang [-Wwrite-strings]: security/selinux/ss/policydb.c:1881:26: error: passing 'const char [8]' to parameter of type 'char *' discards qualifiers hash_eval(&p->range_tr, rangetr); ^~~~~~~~~ security/selinux/ss/policydb.c:707:55: note: passing argument to parameter 'hash_name' here static inline void hash_eval(struct hashtab *h, char *hash_name) ^ security/selinux/ss/policydb.c:2099:32: error: passing 'const char [11]' to parameter of type 'char *' discards qualifiers hash_eval(&p->filename_trans, filenametr); ^~~~~~~~~~~~ security/selinux/ss/policydb.c:707:55: note: passing argument to parameter 'hash_name' here static inline void hash_eval(struct hashtab *h, char *hash_name) ^ Signed-off-by: Christian Göttsche [PM: line wrapping in description] Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 0ae1b718194a..67e03f6e8966 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -704,7 +704,7 @@ static void symtab_hash_eval(struct symtab *s) } #else -static inline void hash_eval(struct hashtab *h, char *hash_name) +static inline void hash_eval(struct hashtab *h, const char *hash_name) { } #endif -- cgit v1.2.3 From 9e2fe574c02bde46307255467a5e4291f65227fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:14 +0100 Subject: selinux: enclose macro arguments in parenthesis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enclose the macro arguments in parenthesis to avoid potential evaluation order issues. Note the xperm and ebitmap macros are still not side-effect safe due to double evaluation. Reported by clang-tidy [bugprone-macro-parentheses] Signed-off-by: Christian Göttsche Reviewed-by: Nick Desaulniers Signed-off-by: Paul Moore --- security/selinux/include/security.h | 4 ++-- security/selinux/ss/ebitmap.h | 6 +++--- security/selinux/ss/sidtab.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 6482e0efb368..d91a5672de99 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -254,8 +254,8 @@ struct av_decision { #define XPERMS_AUDITALLOW 2 #define XPERMS_DONTAUDIT 4 -#define security_xperm_set(perms, x) (perms[x >> 5] |= 1 << (x & 0x1f)) -#define security_xperm_test(perms, x) (1 & (perms[x >> 5] >> (x & 0x1f))) +#define security_xperm_set(perms, x) ((perms)[(x) >> 5] |= 1 << ((x) & 0x1f)) +#define security_xperm_test(perms, x) (1 & ((perms)[(x) >> 5] >> ((x) & 0x1f))) struct extended_perms_data { u32 p[8]; }; diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index 9eb2d0af2805..58eb822f11ee 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -118,9 +118,9 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n, } #define ebitmap_for_each_positive_bit(e, n, bit) \ - for (bit = ebitmap_start_positive(e, &n); \ - bit < ebitmap_length(e); \ - bit = ebitmap_next_positive(e, &n, bit)) \ + for ((bit) = ebitmap_start_positive(e, &(n)); \ + (bit) < ebitmap_length(e); \ + (bit) = ebitmap_next_positive(e, &(n), bit)) \ int ebitmap_cmp(struct ebitmap *e1, struct ebitmap *e2); int ebitmap_cpy(struct ebitmap *dst, struct ebitmap *src); diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index 293ec048af08..a54b8652bfb5 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -27,8 +27,8 @@ struct sidtab_str_cache { char str[]; }; -#define index_to_sid(index) (index + SECINITSID_NUM + 1) -#define sid_to_index(sid) (sid - (SECINITSID_NUM + 1)) +#define index_to_sid(index) ((index) + SECINITSID_NUM + 1) +#define sid_to_index(sid) ((sid) - (SECINITSID_NUM + 1)) int sidtab_init(struct sidtab *s) { -- cgit v1.2.3 From 0b3c2b3dc96a57fd64691d666c4c7123a4f4e7b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:15 +0100 Subject: selinux: drop cast to same type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the lvalue scontextp and rvalue scontext are of the type char*. Drop the redundant explicit cast not needed since commit 9a59daa03df7 ("SELinux: fix sleeping allocation in security_context_to_sid"), where the type of scontext changed from const char* to char*. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 5a7df45bdab1..2f8db93e53b2 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1452,7 +1452,7 @@ static int string_to_context_struct(struct policydb *pol, /* Parse the security context. */ rc = -EINVAL; - scontextp = (char *) scontext; + scontextp = scontext; /* Extract the user. */ p = scontextp; -- cgit v1.2.3 From 056945a96cf58060560498e069a10d94a1ef802b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:16 +0100 Subject: selinux: drop unused parameter of avtab_insert_node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parameter cur is not used in avtab_insert_node(). Reported by clang [-Wunused-parameter] Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/avtab.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index c97695ae508f..cfdae20792e1 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -67,7 +67,7 @@ static inline int avtab_hash(const struct avtab_key *keyp, u32 mask) static struct avtab_node* avtab_insert_node(struct avtab *h, int hvalue, - struct avtab_node *prev, struct avtab_node *cur, + struct avtab_node *prev, const struct avtab_key *key, const struct avtab_datum *datum) { struct avtab_node *newnode; @@ -137,7 +137,7 @@ static int avtab_insert(struct avtab *h, const struct avtab_key *key, break; } - newnode = avtab_insert_node(h, hvalue, prev, cur, key, datum); + newnode = avtab_insert_node(h, hvalue, prev, key, datum); if (!newnode) return -ENOMEM; @@ -177,7 +177,7 @@ struct avtab_node *avtab_insert_nonunique(struct avtab *h, key->target_class < cur->key.target_class) break; } - return avtab_insert_node(h, hvalue, prev, cur, key, datum); + return avtab_insert_node(h, hvalue, prev, key, datum); } struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key *key) -- cgit v1.2.3 From 73073d956a2073554b99d621a7a7ec9437055044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:17 +0100 Subject: selinux: do not discard const qualifier in cast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not discard the const qualifier on the cast from const void* to __be32*; the addressed value is not modified. Reported by clang [-Wcast-qual] Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/netnode.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c index 4a7d2ab5b960..889552db0d31 100644 --- a/security/selinux/netnode.c +++ b/security/selinux/netnode.c @@ -107,7 +107,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family) switch (family) { case PF_INET: - idx = sel_netnode_hashfn_ipv4(*(__be32 *)addr); + idx = sel_netnode_hashfn_ipv4(*(const __be32 *)addr); break; case PF_INET6: idx = sel_netnode_hashfn_ipv6(addr); @@ -121,7 +121,7 @@ static struct sel_netnode *sel_netnode_find(const void *addr, u16 family) if (node->nsec.family == family) switch (family) { case PF_INET: - if (node->nsec.addr.ipv4 == *(__be32 *)addr) + if (node->nsec.addr.ipv4 == *(const __be32 *)addr) return node; break; case PF_INET6: -- cgit v1.2.3 From b084e189b01a7614d3098aca4f2381a759460d88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:18 +0100 Subject: selinux: simplify cred_init_security MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parameter of selinux_cred() is declared const, so an explicit cast dropping the const qualifier is not necessary. Without the cast the local variable cred serves no purpose. Reported by clang [-Wcast-qual] Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/hooks.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index a0243bae8423..eae7dbd62df1 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -211,10 +211,9 @@ static int selinux_lsm_notifier_avc_callback(u32 event) */ static void cred_init_security(void) { - struct cred *cred = (struct cred *) current->real_cred; struct task_security_struct *tsec; - tsec = selinux_cred(cred); + tsec = selinux_cred(current->real_cred); tsec->osid = tsec->sid = SECINITSID_KERNEL; } -- cgit v1.2.3 From b5e68162f859132af419af479bdb96e2ae18fa2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Tue, 25 Jan 2022 15:14:19 +0100 Subject: selinux: drop unused macro MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The macro _DEBUG_HASHES is nowhere used. The configuration DEBUG_HASHES enables debugging of the SELinux hash tables, but the with an underscore prefixed macro definition has no direct impact or any documentation. Reported by clang [-Wunused-macros] Signed-off-by: Christian Göttsche Reviewed-by: Nick Desaulniers Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 67e03f6e8966..d036e1238e77 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -41,8 +41,6 @@ #include "mls.h" #include "services.h" -#define _DEBUG_HASHES - #ifdef DEBUG_HASHES static const char *symtab_name[SYM_NUM] = { "common prefixes", -- cgit v1.2.3 From cdeea45422f579b9302e377d1ede29133d3fde8e Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 27 Jan 2022 10:45:59 -0500 Subject: selinux: fix a type cast problem in cred_init_security() In the process of removing an explicit type cast to preserve a cred const qualifier in cred_init_security() we ran into a problem where the task_struct::real_cred field is defined with the "__rcu" attribute but the selinux_cred() function parameter is not, leading to a sparse warning: security/selinux/hooks.c:216:36: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cred const *cred @@ got struct cred const [noderef] __rcu *real_cred As we don't want to add the "__rcu" attribute to the selinux_cred() parameter, we're going to add an explicit cast back to cred_init_security(). Fixes: b084e189b01a ("selinux: simplify cred_init_security") Reported-by: kernel test robot Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index eae7dbd62df1..221e642025f5 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -213,7 +213,7 @@ static void cred_init_security(void) { struct task_security_struct *tsec; - tsec = selinux_cred(current->real_cred); + tsec = selinux_cred(unrcu_pointer(current->real_cred)); tsec->osid = tsec->sid = SECINITSID_KERNEL; } -- cgit v1.2.3 From ecff30575b5ad0eda149aadad247b7f75411fd47 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Thu, 27 Jan 2022 04:51:00 +0000 Subject: LSM: general protection fault in legacy_parse_param The usual LSM hook "bail on fail" scheme doesn't work for cases where a security module may return an error code indicating that it does not recognize an input. In this particular case Smack sees a mount option that it recognizes, and returns 0. A call to a BPF hook follows, which returns -ENOPARAM, which confuses the caller because Smack has processed its data. The SELinux hook incorrectly returns 1 on success. There was a time when this was correct, however the current expectation is that it return 0 on success. This is repaired. Reported-by: syzbot+d1e3b1d92d25abf97943@syzkaller.appspotmail.com Signed-off-by: Casey Schaufler Acked-by: James Morris Signed-off-by: Paul Moore --- security/security.c | 17 +++++++++++++++-- security/selinux/hooks.c | 5 ++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/security/security.c b/security/security.c index 3d4eb474f35b..e649c8691be2 100644 --- a/security/security.c +++ b/security/security.c @@ -884,9 +884,22 @@ int security_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc) return call_int_hook(fs_context_dup, 0, fc, src_fc); } -int security_fs_context_parse_param(struct fs_context *fc, struct fs_parameter *param) +int security_fs_context_parse_param(struct fs_context *fc, + struct fs_parameter *param) { - return call_int_hook(fs_context_parse_param, -ENOPARAM, fc, param); + struct security_hook_list *hp; + int trc; + int rc = -ENOPARAM; + + hlist_for_each_entry(hp, &security_hook_heads.fs_context_parse_param, + list) { + trc = hp->hook.fs_context_parse_param(fc, param); + if (trc == 0) + rc = 0; + else if (trc != -ENOPARAM) + return trc; + } + return rc; } int security_sb_alloc(struct super_block *sb) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 221e642025f5..9e3658e9e7ca 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2859,10 +2859,9 @@ static int selinux_fs_context_parse_param(struct fs_context *fc, return opt; rc = selinux_add_opt(opt, param->string, &fc->security); - if (!rc) { + if (!rc) param->string = NULL; - rc = 1; - } + return rc; } -- cgit v1.2.3 From b8b87fd954b4b1bdd2d739c8f50bf685351a1a94 Mon Sep 17 00:00:00 2001 From: Scott Mayhew Date: Mon, 31 Jan 2022 13:57:36 -0500 Subject: selinux: Fix selinux_sb_mnt_opts_compat() selinux_sb_mnt_opts_compat() is called under the sb_lock spinlock and shouldn't be performing any memory allocations. Fix this by parsing the sids at the same time we're chopping up the security mount options string and then using the pre-parsed sids when doing the comparison. Fixes: cc274ae7763d ("selinux: fix sleeping function called from invalid context") Fixes: 69c4a42d72eb ("lsm,selinux: add new hook to compare new mount to an existing mount") Signed-off-by: Scott Mayhew Signed-off-by: Paul Moore --- security/selinux/hooks.c | 75 ++++++++++++++++++++++++++---------------------- 1 file changed, 41 insertions(+), 34 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9e3658e9e7ca..85282ccc748f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -341,6 +341,10 @@ static void inode_free_security(struct inode *inode) struct selinux_mnt_opts { const char *fscontext, *context, *rootcontext, *defcontext; + u32 fscontext_sid; + u32 context_sid; + u32 rootcontext_sid; + u32 defcontext_sid; }; static void selinux_free_mnt_opts(void *mnt_opts) @@ -597,15 +601,14 @@ static int bad_option(struct superblock_security_struct *sbsec, char flag, return 0; } -static int parse_sid(struct super_block *sb, const char *s, u32 *sid, - gfp_t gfp) +static int parse_sid(struct super_block *sb, const char *s, u32 *sid) { int rc = security_context_str_to_sid(&selinux_state, s, - sid, gfp); + sid, GFP_KERNEL); if (rc) pr_warn("SELinux: security_context_str_to_sid" "(%s) failed for (dev %s, type %s) errno=%d\n", - s, sb->s_id, sb->s_type->name, rc); + s, sb ? sb->s_id : "?", sb ? sb->s_type->name : "?", rc); return rc; } @@ -672,8 +675,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, */ if (opts) { if (opts->fscontext) { - rc = parse_sid(sb, opts->fscontext, &fscontext_sid, - GFP_KERNEL); + rc = parse_sid(sb, opts->fscontext, &fscontext_sid); if (rc) goto out; if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, @@ -682,8 +684,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, sbsec->flags |= FSCONTEXT_MNT; } if (opts->context) { - rc = parse_sid(sb, opts->context, &context_sid, - GFP_KERNEL); + rc = parse_sid(sb, opts->context, &context_sid); if (rc) goto out; if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, @@ -692,8 +693,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, sbsec->flags |= CONTEXT_MNT; } if (opts->rootcontext) { - rc = parse_sid(sb, opts->rootcontext, &rootcontext_sid, - GFP_KERNEL); + rc = parse_sid(sb, opts->rootcontext, &rootcontext_sid); if (rc) goto out; if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, @@ -702,8 +702,7 @@ static int selinux_set_mnt_opts(struct super_block *sb, sbsec->flags |= ROOTCONTEXT_MNT; } if (opts->defcontext) { - rc = parse_sid(sb, opts->defcontext, &defcontext_sid, - GFP_KERNEL); + rc = parse_sid(sb, opts->defcontext, &defcontext_sid); if (rc) goto out; if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, @@ -995,21 +994,29 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) if (opts->context || opts->defcontext) goto err; opts->context = s; + if (selinux_initialized(&selinux_state)) + parse_sid(NULL, s, &opts->context_sid); break; case Opt_fscontext: if (opts->fscontext) goto err; opts->fscontext = s; + if (selinux_initialized(&selinux_state)) + parse_sid(NULL, s, &opts->fscontext_sid); break; case Opt_rootcontext: if (opts->rootcontext) goto err; opts->rootcontext = s; + if (selinux_initialized(&selinux_state)) + parse_sid(NULL, s, &opts->rootcontext_sid); break; case Opt_defcontext: if (opts->context || opts->defcontext) goto err; opts->defcontext = s; + if (selinux_initialized(&selinux_state)) + parse_sid(NULL, s, &opts->defcontext_sid); break; } @@ -2647,8 +2654,6 @@ static int selinux_sb_mnt_opts_compat(struct super_block *sb, void *mnt_opts) { struct selinux_mnt_opts *opts = mnt_opts; struct superblock_security_struct *sbsec = selinux_superblock(sb); - u32 sid; - int rc; /* * Superblock not initialized (i.e. no options) - reject if any @@ -2665,34 +2670,36 @@ static int selinux_sb_mnt_opts_compat(struct super_block *sb, void *mnt_opts) return (sbsec->flags & SE_MNTMASK) ? 1 : 0; if (opts->fscontext) { - rc = parse_sid(sb, opts->fscontext, &sid, GFP_NOWAIT); - if (rc) + if (opts->fscontext_sid == SECSID_NULL) return 1; - if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, sid)) + else if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, + opts->fscontext_sid)) return 1; } if (opts->context) { - rc = parse_sid(sb, opts->context, &sid, GFP_NOWAIT); - if (rc) + if (opts->context_sid == SECSID_NULL) return 1; - if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, sid)) + else if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, + opts->context_sid)) return 1; } if (opts->rootcontext) { - struct inode_security_struct *root_isec; - - root_isec = backing_inode_security(sb->s_root); - rc = parse_sid(sb, opts->rootcontext, &sid, GFP_NOWAIT); - if (rc) - return 1; - if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, sid)) + if (opts->rootcontext_sid == SECSID_NULL) return 1; + else { + struct inode_security_struct *root_isec; + + root_isec = backing_inode_security(sb->s_root); + if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, + opts->rootcontext_sid)) + return 1; + } } if (opts->defcontext) { - rc = parse_sid(sb, opts->defcontext, &sid, GFP_NOWAIT); - if (rc) + if (opts->defcontext_sid == SECSID_NULL) return 1; - if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, sid)) + else if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, + opts->defcontext_sid)) return 1; } return 0; @@ -2712,14 +2719,14 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) return 0; if (opts->fscontext) { - rc = parse_sid(sb, opts->fscontext, &sid, GFP_KERNEL); + rc = parse_sid(sb, opts->fscontext, &sid); if (rc) return rc; if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, sid)) goto out_bad_option; } if (opts->context) { - rc = parse_sid(sb, opts->context, &sid, GFP_KERNEL); + rc = parse_sid(sb, opts->context, &sid); if (rc) return rc; if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, sid)) @@ -2728,14 +2735,14 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) if (opts->rootcontext) { struct inode_security_struct *root_isec; root_isec = backing_inode_security(sb->s_root); - rc = parse_sid(sb, opts->rootcontext, &sid, GFP_KERNEL); + rc = parse_sid(sb, opts->rootcontext, &sid); if (rc) return rc; if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, sid)) goto out_bad_option; } if (opts->defcontext) { - rc = parse_sid(sb, opts->defcontext, &sid, GFP_KERNEL); + rc = parse_sid(sb, opts->defcontext, &sid); if (rc) return rc; if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, sid)) -- cgit v1.2.3 From 6bc1968c14e91e03c0851b9c5c5330d91305a853 Mon Sep 17 00:00:00 2001 From: Scott Mayhew Date: Mon, 31 Jan 2022 13:57:37 -0500 Subject: selinux: try to use preparsed sid before calling parse_sid() Avoid unnecessary parsing of sids that have already been parsed via selinux_sb_eat_lsm_opts(). Signed-off-by: Scott Mayhew Signed-off-by: Paul Moore --- security/selinux/hooks.c | 88 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 85282ccc748f..b60481192b38 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -675,36 +675,48 @@ static int selinux_set_mnt_opts(struct super_block *sb, */ if (opts) { if (opts->fscontext) { - rc = parse_sid(sb, opts->fscontext, &fscontext_sid); - if (rc) - goto out; + if (opts->fscontext_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->fscontext, &fscontext_sid); + if (rc) + goto out; + } else + fscontext_sid = opts->fscontext_sid; if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, fscontext_sid)) goto out_double_mount; sbsec->flags |= FSCONTEXT_MNT; } if (opts->context) { - rc = parse_sid(sb, opts->context, &context_sid); - if (rc) - goto out; + if (opts->context_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->context, &context_sid); + if (rc) + goto out; + } else + context_sid = opts->context_sid; if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, context_sid)) goto out_double_mount; sbsec->flags |= CONTEXT_MNT; } if (opts->rootcontext) { - rc = parse_sid(sb, opts->rootcontext, &rootcontext_sid); - if (rc) - goto out; + if (opts->rootcontext_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->rootcontext, &rootcontext_sid); + if (rc) + goto out; + } else + rootcontext_sid = opts->rootcontext_sid; if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, rootcontext_sid)) goto out_double_mount; sbsec->flags |= ROOTCONTEXT_MNT; } if (opts->defcontext) { - rc = parse_sid(sb, opts->defcontext, &defcontext_sid); - if (rc) - goto out; + if (opts->defcontext_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->defcontext, &defcontext_sid); + if (rc) + goto out; + } else + defcontext_sid = opts->defcontext_sid; if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, defcontext_sid)) goto out_double_mount; @@ -2709,7 +2721,6 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) { struct selinux_mnt_opts *opts = mnt_opts; struct superblock_security_struct *sbsec = selinux_superblock(sb); - u32 sid; int rc; if (!(sbsec->flags & SE_SBINITIALIZED)) @@ -2719,33 +2730,48 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) return 0; if (opts->fscontext) { - rc = parse_sid(sb, opts->fscontext, &sid); - if (rc) - return rc; - if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, sid)) + if (opts->fscontext_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->fscontext, + &opts->fscontext_sid); + if (rc) + return rc; + } + if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, + opts->fscontext_sid)) goto out_bad_option; } if (opts->context) { - rc = parse_sid(sb, opts->context, &sid); - if (rc) - return rc; - if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, sid)) + if (opts->context_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->context, &opts->context_sid); + if (rc) + return rc; + } + if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, + opts->context_sid)) goto out_bad_option; } if (opts->rootcontext) { struct inode_security_struct *root_isec; root_isec = backing_inode_security(sb->s_root); - rc = parse_sid(sb, opts->rootcontext, &sid); - if (rc) - return rc; - if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, sid)) + if (opts->rootcontext_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->rootcontext, + &opts->rootcontext_sid); + if (rc) + return rc; + } + if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, + opts->rootcontext_sid)) goto out_bad_option; } if (opts->defcontext) { - rc = parse_sid(sb, opts->defcontext, &sid); - if (rc) - return rc; - if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, sid)) + if (opts->defcontext_sid == SECSID_NULL) { + rc = parse_sid(sb, opts->defcontext, + &opts->defcontext_sid); + if (rc) + return rc; + } + if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, + opts->defcontext_sid)) goto out_bad_option; } return 0; @@ -2843,6 +2869,10 @@ static int selinux_fs_context_dup(struct fs_context *fc, if (!opts->defcontext) return -ENOMEM; } + opts->fscontext_sid = src->fscontext_sid; + opts->context_sid = src->context_sid; + opts->rootcontext_sid = src->rootcontext_sid; + opts->defcontext_sid = src->defcontext_sid; return 0; } -- cgit v1.2.3 From 0e326df069802dc48e4f095f889cb780e4beaba6 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 27 Jan 2022 10:56:13 -0500 Subject: selinux: various sparse fixes When running the SELinux code through sparse, there are a handful of warnings. This patch resolves some of these warnings caused by "__rcu" mismatches. % make W=1 C=1 security/selinux/ Signed-off-by: Paul Moore --- security/selinux/hooks.c | 6 +++--- security/selinux/ibpkey.c | 2 +- security/selinux/netnode.c | 5 +++-- security/selinux/netport.c | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index b60481192b38..b24a1aeeedd4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2553,7 +2553,7 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm) if (rc) { clear_itimer(); - spin_lock_irq(¤t->sighand->siglock); + spin_lock_irq(&unrcu_pointer(current->sighand)->siglock); if (!fatal_signal_pending(current)) { flush_sigqueue(¤t->pending); flush_sigqueue(¤t->signal->shared_pending); @@ -2561,13 +2561,13 @@ static void selinux_bprm_committed_creds(struct linux_binprm *bprm) sigemptyset(¤t->blocked); recalc_sigpending(); } - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irq(&unrcu_pointer(current->sighand)->siglock); } /* Wake up the parent if it is waiting so that it can recheck * wait permission to the new task SID. */ read_lock(&tasklist_lock); - __wake_up_parent(current, current->real_parent); + __wake_up_parent(current, unrcu_pointer(current->real_parent)); read_unlock(&tasklist_lock); } diff --git a/security/selinux/ibpkey.c b/security/selinux/ibpkey.c index 20b3b2243820..5839ca7bb9c7 100644 --- a/security/selinux/ibpkey.c +++ b/security/selinux/ibpkey.c @@ -104,7 +104,7 @@ static void sel_ib_pkey_insert(struct sel_ib_pkey *pkey) tail = list_entry( rcu_dereference_protected( - sel_ib_pkey_hash[idx].list.prev, + list_tail_rcu(&sel_ib_pkey_hash[idx].list), lockdep_is_held(&sel_ib_pkey_lock)), struct sel_ib_pkey, list); list_del_rcu(&tail->list); diff --git a/security/selinux/netnode.c b/security/selinux/netnode.c index 889552db0d31..0ac7df9a9367 100644 --- a/security/selinux/netnode.c +++ b/security/selinux/netnode.c @@ -164,8 +164,9 @@ static void sel_netnode_insert(struct sel_netnode *node) if (sel_netnode_hash[idx].size == SEL_NETNODE_HASH_BKT_LIMIT) { struct sel_netnode *tail; tail = list_entry( - rcu_dereference_protected(sel_netnode_hash[idx].list.prev, - lockdep_is_held(&sel_netnode_lock)), + rcu_dereference_protected( + list_tail_rcu(&sel_netnode_hash[idx].list), + lockdep_is_held(&sel_netnode_lock)), struct sel_netnode, list); list_del_rcu(&tail->list); kfree_rcu(tail, rcu); diff --git a/security/selinux/netport.c b/security/selinux/netport.c index 9ba09d11c0f5..8eec6347cf01 100644 --- a/security/selinux/netport.c +++ b/security/selinux/netport.c @@ -113,7 +113,7 @@ static void sel_netport_insert(struct sel_netport *port) struct sel_netport *tail; tail = list_entry( rcu_dereference_protected( - sel_netport_hash[idx].list.prev, + list_tail_rcu(&sel_netport_hash[idx].list), lockdep_is_held(&sel_netport_lock)), struct sel_netport, list); list_del_rcu(&tail->list); -- cgit v1.2.3 From 70f4169ab421b277caf7429e84f468d8c47aa00a Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Wed, 2 Feb 2022 13:55:29 +0100 Subject: selinux: parse contexts for mount options early Commit b8b87fd954b4 ("selinux: Fix selinux_sb_mnt_opts_compat()") started to parse mount options into SIDs in selinux_add_opt() if policy has already been loaded. Since it's extremely unlikely that anyone would depend on the ability to set SELinux contexts on fs_context before loading the policy and then mounting that context after simplify the logic by always parsing the options early. Note that the multi-step mounting is only possible with the new fscontext mount API and wasn't possible before its introduction. Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 202 +++++++++++++---------------------------------- 1 file changed, 53 insertions(+), 149 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index b24a1aeeedd4..ab32303e6618 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -340,7 +340,6 @@ static void inode_free_security(struct inode *inode) } struct selinux_mnt_opts { - const char *fscontext, *context, *rootcontext, *defcontext; u32 fscontext_sid; u32 context_sid; u32 rootcontext_sid; @@ -349,12 +348,7 @@ struct selinux_mnt_opts { static void selinux_free_mnt_opts(void *mnt_opts) { - struct selinux_mnt_opts *opts = mnt_opts; - kfree(opts->fscontext); - kfree(opts->context); - kfree(opts->rootcontext); - kfree(opts->defcontext); - kfree(opts); + kfree(mnt_opts); } enum { @@ -601,17 +595,6 @@ static int bad_option(struct superblock_security_struct *sbsec, char flag, return 0; } -static int parse_sid(struct super_block *sb, const char *s, u32 *sid) -{ - int rc = security_context_str_to_sid(&selinux_state, s, - sid, GFP_KERNEL); - if (rc) - pr_warn("SELinux: security_context_str_to_sid" - "(%s) failed for (dev %s, type %s) errno=%d\n", - s, sb ? sb->s_id : "?", sb ? sb->s_type->name : "?", rc); - return rc; -} - /* * Allow filesystems with binary mount data to explicitly set mount point * labeling information. @@ -674,49 +657,29 @@ static int selinux_set_mnt_opts(struct super_block *sb, * than once with different security options. */ if (opts) { - if (opts->fscontext) { - if (opts->fscontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->fscontext, &fscontext_sid); - if (rc) - goto out; - } else - fscontext_sid = opts->fscontext_sid; + if (opts->fscontext_sid) { + fscontext_sid = opts->fscontext_sid; if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, fscontext_sid)) goto out_double_mount; sbsec->flags |= FSCONTEXT_MNT; } - if (opts->context) { - if (opts->context_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->context, &context_sid); - if (rc) - goto out; - } else - context_sid = opts->context_sid; + if (opts->context_sid) { + context_sid = opts->context_sid; if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, context_sid)) goto out_double_mount; sbsec->flags |= CONTEXT_MNT; } - if (opts->rootcontext) { - if (opts->rootcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->rootcontext, &rootcontext_sid); - if (rc) - goto out; - } else - rootcontext_sid = opts->rootcontext_sid; + if (opts->rootcontext_sid) { + rootcontext_sid = opts->rootcontext_sid; if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, rootcontext_sid)) goto out_double_mount; sbsec->flags |= ROOTCONTEXT_MNT; } - if (opts->defcontext) { - if (opts->defcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->defcontext, &defcontext_sid); - if (rc) - goto out; - } else - defcontext_sid = opts->defcontext_sid; + if (opts->defcontext_sid) { + defcontext_sid = opts->defcontext_sid; if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, defcontext_sid)) goto out_double_mount; @@ -986,6 +949,8 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) { struct selinux_mnt_opts *opts = *mnt_opts; bool is_alloc_opts = false; + u32 *dst_sid; + int rc; if (token == Opt_seclabel) /* eaten and completely ignored */ @@ -993,6 +958,11 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) if (!s) return -ENOMEM; + if (!selinux_initialized(&selinux_state)) { + pr_warn("SELinux: Unable to set superblock options before the security server is initialized\n"); + return -EINVAL; + } + if (!opts) { opts = kzalloc(sizeof(*opts), GFP_KERNEL); if (!opts) @@ -1003,36 +973,34 @@ static int selinux_add_opt(int token, const char *s, void **mnt_opts) switch (token) { case Opt_context: - if (opts->context || opts->defcontext) + if (opts->context_sid || opts->defcontext_sid) goto err; - opts->context = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->context_sid); + dst_sid = &opts->context_sid; break; case Opt_fscontext: - if (opts->fscontext) + if (opts->fscontext_sid) goto err; - opts->fscontext = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->fscontext_sid); + dst_sid = &opts->fscontext_sid; break; case Opt_rootcontext: - if (opts->rootcontext) + if (opts->rootcontext_sid) goto err; - opts->rootcontext = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->rootcontext_sid); + dst_sid = &opts->rootcontext_sid; break; case Opt_defcontext: - if (opts->context || opts->defcontext) + if (opts->context_sid || opts->defcontext_sid) goto err; - opts->defcontext = s; - if (selinux_initialized(&selinux_state)) - parse_sid(NULL, s, &opts->defcontext_sid); + dst_sid = &opts->defcontext_sid; break; + default: + WARN_ON(1); + return -EINVAL; } - - return 0; + rc = security_context_str_to_sid(&selinux_state, s, dst_sid, GFP_KERNEL); + if (rc) + pr_warn("SELinux: security_context_str_to_sid (%s) failed with errno=%d\n", + s, rc); + return rc; err: if (is_alloc_opts) { @@ -2681,37 +2649,27 @@ static int selinux_sb_mnt_opts_compat(struct super_block *sb, void *mnt_opts) if (!opts) return (sbsec->flags & SE_MNTMASK) ? 1 : 0; - if (opts->fscontext) { - if (opts->fscontext_sid == SECSID_NULL) - return 1; - else if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, - opts->fscontext_sid)) + if (opts->fscontext_sid) { + if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, + opts->fscontext_sid)) return 1; } - if (opts->context) { - if (opts->context_sid == SECSID_NULL) - return 1; - else if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, - opts->context_sid)) + if (opts->context_sid) { + if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, + opts->context_sid)) return 1; } - if (opts->rootcontext) { - if (opts->rootcontext_sid == SECSID_NULL) - return 1; - else { - struct inode_security_struct *root_isec; + if (opts->rootcontext_sid) { + struct inode_security_struct *root_isec; - root_isec = backing_inode_security(sb->s_root); - if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, - opts->rootcontext_sid)) - return 1; - } - } - if (opts->defcontext) { - if (opts->defcontext_sid == SECSID_NULL) + root_isec = backing_inode_security(sb->s_root); + if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, + opts->rootcontext_sid)) return 1; - else if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, - opts->defcontext_sid)) + } + if (opts->defcontext_sid) { + if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, + opts->defcontext_sid)) return 1; } return 0; @@ -2721,7 +2679,6 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) { struct selinux_mnt_opts *opts = mnt_opts; struct superblock_security_struct *sbsec = selinux_superblock(sb); - int rc; if (!(sbsec->flags & SE_SBINITIALIZED)) return 0; @@ -2729,47 +2686,24 @@ static int selinux_sb_remount(struct super_block *sb, void *mnt_opts) if (!opts) return 0; - if (opts->fscontext) { - if (opts->fscontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->fscontext, - &opts->fscontext_sid); - if (rc) - return rc; - } + if (opts->fscontext_sid) { if (bad_option(sbsec, FSCONTEXT_MNT, sbsec->sid, opts->fscontext_sid)) goto out_bad_option; } - if (opts->context) { - if (opts->context_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->context, &opts->context_sid); - if (rc) - return rc; - } + if (opts->context_sid) { if (bad_option(sbsec, CONTEXT_MNT, sbsec->mntpoint_sid, opts->context_sid)) goto out_bad_option; } - if (opts->rootcontext) { + if (opts->rootcontext_sid) { struct inode_security_struct *root_isec; root_isec = backing_inode_security(sb->s_root); - if (opts->rootcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->rootcontext, - &opts->rootcontext_sid); - if (rc) - return rc; - } if (bad_option(sbsec, ROOTCONTEXT_MNT, root_isec->sid, opts->rootcontext_sid)) goto out_bad_option; } - if (opts->defcontext) { - if (opts->defcontext_sid == SECSID_NULL) { - rc = parse_sid(sb, opts->defcontext, - &opts->defcontext_sid); - if (rc) - return rc; - } + if (opts->defcontext_sid) { if (bad_option(sbsec, DEFCONTEXT_MNT, sbsec->def_sid, opts->defcontext_sid)) goto out_bad_option; @@ -2838,42 +2772,12 @@ static int selinux_fs_context_dup(struct fs_context *fc, struct fs_context *src_fc) { const struct selinux_mnt_opts *src = src_fc->security; - struct selinux_mnt_opts *opts; if (!src) return 0; - fc->security = kzalloc(sizeof(struct selinux_mnt_opts), GFP_KERNEL); - if (!fc->security) - return -ENOMEM; - - opts = fc->security; - - if (src->fscontext) { - opts->fscontext = kstrdup(src->fscontext, GFP_KERNEL); - if (!opts->fscontext) - return -ENOMEM; - } - if (src->context) { - opts->context = kstrdup(src->context, GFP_KERNEL); - if (!opts->context) - return -ENOMEM; - } - if (src->rootcontext) { - opts->rootcontext = kstrdup(src->rootcontext, GFP_KERNEL); - if (!opts->rootcontext) - return -ENOMEM; - } - if (src->defcontext) { - opts->defcontext = kstrdup(src->defcontext, GFP_KERNEL); - if (!opts->defcontext) - return -ENOMEM; - } - opts->fscontext_sid = src->fscontext_sid; - opts->context_sid = src->context_sid; - opts->rootcontext_sid = src->rootcontext_sid; - opts->defcontext_sid = src->defcontext_sid; - return 0; + fc->security = kmemdup(src, sizeof(*src), GFP_KERNEL); + return fc->security ? 0 : -ENOMEM; } static const struct fs_parameter_spec selinux_fs_parameters[] = { -- cgit v1.2.3 From 5e50f5d4ff31e95599d695df1f0a4e7d2d6fef99 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Sat, 12 Feb 2022 18:59:21 +0100 Subject: security: add sctp_assoc_established hook security_sctp_assoc_established() is added to replace security_inet_conn_established() called in sctp_sf_do_5_1E_ca(), so that asoc can be accessed in security subsystem and save the peer secid to asoc->peer_secid. Fixes: 72e89f50084c ("security: Add support for SCTP security hooks") Reported-by: Prashanth Prahlad Based-on-patch-by: Xin Long Reviewed-by: Xin Long Tested-by: Richard Haines Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- Documentation/security/SCTP.rst | 22 ++++++++++------------ include/linux/lsm_hook_defs.h | 2 ++ include/linux/lsm_hooks.h | 5 +++++ include/linux/security.h | 8 ++++++++ net/sctp/sm_statefuns.c | 8 +++++--- security/security.c | 7 +++++++ 6 files changed, 37 insertions(+), 15 deletions(-) diff --git a/Documentation/security/SCTP.rst b/Documentation/security/SCTP.rst index d5fd6ccc3dcb..406cc68b8808 100644 --- a/Documentation/security/SCTP.rst +++ b/Documentation/security/SCTP.rst @@ -15,10 +15,7 @@ For security module support, three SCTP specific hooks have been implemented:: security_sctp_assoc_request() security_sctp_bind_connect() security_sctp_sk_clone() - -Also the following security hook has been utilised:: - - security_inet_conn_established() + security_sctp_assoc_established() The usage of these hooks are described below with the SELinux implementation described in the `SCTP SELinux Support`_ chapter. @@ -122,11 +119,12 @@ calls **sctp_peeloff**\(3). @newsk - pointer to new sock structure. -security_inet_conn_established() +security_sctp_assoc_established() ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Called when a COOKIE ACK is received:: +Called when a COOKIE ACK is received, and the peer secid will be +saved into ``@asoc->peer_secid`` for client:: - @sk - pointer to sock structure. + @asoc - pointer to sctp association structure. @skb - pointer to skbuff of the COOKIE ACK packet. @@ -134,7 +132,7 @@ Security Hooks used for Association Establishment ------------------------------------------------- The following diagram shows the use of ``security_sctp_bind_connect()``, -``security_sctp_assoc_request()``, ``security_inet_conn_established()`` when +``security_sctp_assoc_request()``, ``security_sctp_assoc_established()`` when establishing an association. :: @@ -172,7 +170,7 @@ establishing an association. <------------------------------------------- COOKIE ACK | | sctp_sf_do_5_1E_ca | - Call security_inet_conn_established() | + Call security_sctp_assoc_established() | to set the peer label. | | | | If SCTP_SOCKET_TCP or peeled off @@ -198,7 +196,7 @@ hooks with the SELinux specifics expanded below:: security_sctp_assoc_request() security_sctp_bind_connect() security_sctp_sk_clone() - security_inet_conn_established() + security_sctp_assoc_established() security_sctp_assoc_request() @@ -271,12 +269,12 @@ sockets sid and peer sid to that contained in the ``@asoc sid`` and @newsk - pointer to new sock structure. -security_inet_conn_established() +security_sctp_assoc_established() ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Called when a COOKIE ACK is received where it sets the connection's peer sid to that in ``@skb``:: - @sk - pointer to sock structure. + @asoc - pointer to sctp association structure. @skb - pointer to skbuff of the COOKIE ACK packet. diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index a5a724c308d8..45931d81ccc3 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -332,6 +332,8 @@ LSM_HOOK(int, 0, sctp_bind_connect, struct sock *sk, int optname, struct sockaddr *address, int addrlen) LSM_HOOK(void, LSM_RET_VOID, sctp_sk_clone, struct sctp_association *asoc, struct sock *sk, struct sock *newsk) +LSM_HOOK(int, 0, sctp_assoc_established, struct sctp_association *asoc, + struct sk_buff *skb) #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 3bf5c658bc44..419b5febc3ca 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1046,6 +1046,11 @@ * @asoc pointer to current sctp association structure. * @sk pointer to current sock structure. * @newsk pointer to new sock structure. + * @sctp_assoc_established: + * Passes the @asoc and @chunk->skb of the association COOKIE_ACK packet + * to the security module. + * @asoc pointer to sctp association structure. + * @skb pointer to skbuff of association packet. * * Security hooks for Infiniband * diff --git a/include/linux/security.h b/include/linux/security.h index 6d72772182c8..25b3ef71f495 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1422,6 +1422,8 @@ int security_sctp_bind_connect(struct sock *sk, int optname, struct sockaddr *address, int addrlen); void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk, struct sock *newsk); +int security_sctp_assoc_established(struct sctp_association *asoc, + struct sk_buff *skb); #else /* CONFIG_SECURITY_NETWORK */ static inline int security_unix_stream_connect(struct sock *sock, @@ -1641,6 +1643,12 @@ static inline void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *newsk) { } + +static inline int security_sctp_assoc_established(struct sctp_association *asoc, + struct sk_buff *skb) +{ + return 0; +} #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index cc544a97c4af..7f342bc12735 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -930,6 +930,11 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net, if (!sctp_vtag_verify(chunk, asoc)) return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); + /* Set peer label for connection. */ + if (security_sctp_assoc_established((struct sctp_association *)asoc, + chunk->skb)) + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); + /* Verify that the chunk length for the COOKIE-ACK is OK. * If we don't do this, any bundled chunks may be junked. */ @@ -945,9 +950,6 @@ enum sctp_disposition sctp_sf_do_5_1E_ca(struct net *net, */ sctp_add_cmd_sf(commands, SCTP_CMD_INIT_COUNTER_RESET, SCTP_NULL()); - /* Set peer label for connection. */ - security_inet_conn_established(ep->base.sk, chunk->skb); - /* RFC 2960 5.1 Normal Establishment of an Association * * E) Upon reception of the COOKIE ACK, endpoint "A" will move diff --git a/security/security.c b/security/security.c index e649c8691be2..9663ffcca4b0 100644 --- a/security/security.c +++ b/security/security.c @@ -2393,6 +2393,13 @@ void security_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk, } EXPORT_SYMBOL(security_sctp_sk_clone); +int security_sctp_assoc_established(struct sctp_association *asoc, + struct sk_buff *skb) +{ + return call_int_hook(sctp_assoc_established, 0, asoc, skb); +} +EXPORT_SYMBOL(security_sctp_assoc_established); + #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND -- cgit v1.2.3 From 3eb8eaf2ca3e98d4f6e52bed6148ee8fe3069a3d Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Sat, 12 Feb 2022 18:59:22 +0100 Subject: security: implement sctp_assoc_established hook in selinux Do this by extracting the peer labeling per-association logic from selinux_sctp_assoc_request() into a new helper selinux_sctp_process_new_assoc() and use this helper in both selinux_sctp_assoc_request() and selinux_sctp_assoc_established(). This ensures that the peer labeling behavior as documented in Documentation/security/SCTP.rst is applied both on the client and server side: """ An SCTP socket will only have one peer label assigned to it. This will be assigned during the establishment of the first association. Any further associations on this socket will have their packet peer label compared to the sockets peer label, and only if they are different will the ``association`` permission be validated. This is validated by checking the socket peer sid against the received packets peer sid to determine whether the association should be allowed or denied. """ At the same time, it also ensures that the peer label of the association is set to the correct value, such that if it is peeled off into a new socket, the socket's peer label will then be set to the association's peer label, same as it already works on the server side. While selinux_inet_conn_established() (which we are replacing by selinux_sctp_assoc_established() for SCTP) only deals with assigning a peer label to the connection (socket), in case of SCTP we need to also copy the (local) socket label to the association, so that selinux_sctp_sk_clone() can then pick it up for the new socket in case of SCTP peeloff. Careful readers will notice that the selinux_sctp_process_new_assoc() helper also includes the "IPv4 packet received over an IPv6 socket" check, even though it hadn't been in selinux_sctp_assoc_request() before. While such check is not necessary in selinux_inet_conn_request() (because struct request_sock's family field is already set according to the skb's family), here it is needed, as we don't have request_sock and we take the initial family from the socket. In selinux_sctp_assoc_established() it is similarly needed as well (and also selinux_inet_conn_established() already has it). Fixes: 72e89f50084c ("security: Add support for SCTP security hooks") Reported-by: Prashanth Prahlad Based-on-patch-by: Xin Long Reviewed-by: Xin Long Tested-by: Richard Haines Signed-off-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/hooks.c | 90 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 24 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ab32303e6618..dafabb4dcc64 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5238,37 +5238,38 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent) sksec->sclass = isec->sclass; } -/* Called whenever SCTP receives an INIT chunk. This happens when an incoming - * connect(2), sctp_connectx(3) or sctp_sendmsg(3) (with no association - * already present). +/* + * Determines peer_secid for the asoc and updates socket's peer label + * if it's the first association on the socket. */ -static int selinux_sctp_assoc_request(struct sctp_association *asoc, - struct sk_buff *skb) +static int selinux_sctp_process_new_assoc(struct sctp_association *asoc, + struct sk_buff *skb) { - struct sk_security_struct *sksec = asoc->base.sk->sk_security; + struct sock *sk = asoc->base.sk; + u16 family = sk->sk_family; + struct sk_security_struct *sksec = sk->sk_security; struct common_audit_data ad; struct lsm_network_audit net = {0,}; - u8 peerlbl_active; - u32 peer_sid = SECINITSID_UNLABELED; - u32 conn_sid; - int err = 0; + int err; - if (!selinux_policycap_extsockclass()) - return 0; + /* handle mapped IPv4 packets arriving via IPv6 sockets */ + if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP)) + family = PF_INET; - peerlbl_active = selinux_peerlbl_enabled(); + if (selinux_peerlbl_enabled()) { + asoc->peer_secid = SECSID_NULL; - if (peerlbl_active) { /* This will return peer_sid = SECSID_NULL if there are * no peer labels, see security_net_peersid_resolve(). */ - err = selinux_skb_peerlbl_sid(skb, asoc->base.sk->sk_family, - &peer_sid); + err = selinux_skb_peerlbl_sid(skb, family, &asoc->peer_secid); if (err) return err; - if (peer_sid == SECSID_NULL) - peer_sid = SECINITSID_UNLABELED; + if (asoc->peer_secid == SECSID_NULL) + asoc->peer_secid = SECINITSID_UNLABELED; + } else { + asoc->peer_secid = SECINITSID_UNLABELED; } if (sksec->sctp_assoc_state == SCTP_ASSOC_UNSET) { @@ -5279,8 +5280,8 @@ static int selinux_sctp_assoc_request(struct sctp_association *asoc, * then it is approved by policy and used as the primary * peer SID for getpeercon(3). */ - sksec->peer_sid = peer_sid; - } else if (sksec->peer_sid != peer_sid) { + sksec->peer_sid = asoc->peer_secid; + } else if (sksec->peer_sid != asoc->peer_secid) { /* Other association peer SIDs are checked to enforce * consistency among the peer SIDs. */ @@ -5288,11 +5289,32 @@ static int selinux_sctp_assoc_request(struct sctp_association *asoc, ad.u.net = &net; ad.u.net->sk = asoc->base.sk; err = avc_has_perm(&selinux_state, - sksec->peer_sid, peer_sid, sksec->sclass, - SCTP_SOCKET__ASSOCIATION, &ad); + sksec->peer_sid, asoc->peer_secid, + sksec->sclass, SCTP_SOCKET__ASSOCIATION, + &ad); if (err) return err; } + return 0; +} + +/* Called whenever SCTP receives an INIT or COOKIE ECHO chunk. This + * happens on an incoming connect(2), sctp_connectx(3) or + * sctp_sendmsg(3) (with no association already present). + */ +static int selinux_sctp_assoc_request(struct sctp_association *asoc, + struct sk_buff *skb) +{ + struct sk_security_struct *sksec = asoc->base.sk->sk_security; + u32 conn_sid; + int err; + + if (!selinux_policycap_extsockclass()) + return 0; + + err = selinux_sctp_process_new_assoc(asoc, skb); + if (err) + return err; /* Compute the MLS component for the connection and store * the information in asoc. This will be used by SCTP TCP type @@ -5300,17 +5322,36 @@ static int selinux_sctp_assoc_request(struct sctp_association *asoc, * socket to be generated. selinux_sctp_sk_clone() will then * plug this into the new socket. */ - err = selinux_conn_sid(sksec->sid, peer_sid, &conn_sid); + err = selinux_conn_sid(sksec->sid, asoc->peer_secid, &conn_sid); if (err) return err; asoc->secid = conn_sid; - asoc->peer_secid = peer_sid; /* Set any NetLabel labels including CIPSO/CALIPSO options. */ return selinux_netlbl_sctp_assoc_request(asoc, skb); } +/* Called when SCTP receives a COOKIE ACK chunk as the final + * response to an association request (initited by us). + */ +static int selinux_sctp_assoc_established(struct sctp_association *asoc, + struct sk_buff *skb) +{ + struct sk_security_struct *sksec = asoc->base.sk->sk_security; + + if (!selinux_policycap_extsockclass()) + return 0; + + /* Inherit secid from the parent socket - this will be picked up + * by selinux_sctp_sk_clone() if the association gets peeled off + * into a new socket. + */ + asoc->secid = sksec->sid; + + return selinux_sctp_process_new_assoc(asoc, skb); +} + /* Check if sctp IPv4/IPv6 addresses are valid for binding or connecting * based on their @optname. */ @@ -7131,6 +7172,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sctp_assoc_request, selinux_sctp_assoc_request), LSM_HOOK_INIT(sctp_sk_clone, selinux_sctp_sk_clone), LSM_HOOK_INIT(sctp_bind_connect, selinux_sctp_bind_connect), + LSM_HOOK_INIT(sctp_assoc_established, selinux_sctp_assoc_established), LSM_HOOK_INIT(inet_conn_request, selinux_inet_conn_request), LSM_HOOK_INIT(inet_csk_clone, selinux_inet_csk_clone), LSM_HOOK_INIT(inet_conn_established, selinux_inet_conn_established), -- cgit v1.2.3 From 5ea33af9d430cd1dbfada1b839e0d317ed77bfac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 17 Feb 2022 15:21:29 +0100 Subject: selinux: drop return statement at end of void functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Those return statements at the end of a void function are redundant. Reported by clang-tidy [readability-redundant-control-flow] Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 -- security/selinux/ss/conditional.c | 2 -- security/selinux/ss/ebitmap.c | 1 - security/selinux/ss/mls.c | 1 - security/selinux/ss/services.c | 2 -- 5 files changed, 8 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index dafabb4dcc64..1e69f88eb326 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3284,8 +3284,6 @@ static void selinux_inode_post_setxattr(struct dentry *dentry, const char *name, isec->sid = newsid; isec->initialized = LABEL_INITIALIZED; spin_unlock(&isec->lock); - - return; } static int selinux_inode_getxattr(struct dentry *dentry, const char *name) diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 2ec6e5cd25d9..c46c419af512 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -566,8 +566,6 @@ void cond_compute_xperms(struct avtab *ctab, struct avtab_key *key, if (node->key.specified & AVTAB_ENABLED) services_compute_xperms_decision(xpermd, node); } - return; - } /* Determine whether additional permissions are granted by the conditional * av table, and if so, add them to the result diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 61fcbb8d0f88..abde349c8321 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -359,7 +359,6 @@ void ebitmap_destroy(struct ebitmap *e) e->highbit = 0; e->node = NULL; - return; } int ebitmap_read(struct ebitmap *e, void *fp) diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index 3f5fd124342c..99571b19d4a9 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -156,7 +156,6 @@ void mls_sid_to_context(struct policydb *p, } *scontext = scontextp; - return; } int mls_level_isvalid(struct policydb *p, struct mls_level *l) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 2f8db93e53b2..6901dc07680d 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -529,8 +529,6 @@ out: /* release scontext/tcontext */ kfree(tcontext_name); kfree(scontext_name); - - return; } /* -- cgit v1.2.3 From b97df7c098c531010e445da88d02b7bf7bf59ef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 17 Feb 2022 15:21:25 +0100 Subject: selinux: use correct type for context length MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit security_sid_to_context() expects a pointer to an u32 as the address where to store the length of the computed context. Reported by sparse: security/selinux/xfrm.c:359:39: warning: incorrect type in arg 4 (different signedness) security/selinux/xfrm.c:359:39: expected unsigned int [usertype] *scontext_len security/selinux/xfrm.c:359:39: got int * Signed-off-by: Christian Göttsche [PM: wrapped commit description] Signed-off-by: Paul Moore --- security/selinux/xfrm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 90697317895f..c576832febc6 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -347,7 +347,7 @@ int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, int rc; struct xfrm_sec_ctx *ctx; char *ctx_str = NULL; - int str_len; + u32 str_len; if (!polsec) return 0; -- cgit v1.2.3 From 65881e1db4e948614d9eb195b8e1197339822949 Mon Sep 17 00:00:00 2001 From: Richard Haines Date: Fri, 25 Feb 2022 17:54:38 +0000 Subject: selinux: allow FIOCLEX and FIONCLEX with policy capability These ioctls are equivalent to fcntl(fd, F_SETFD, flags), which SELinux always allows too. Furthermore, a failed FIOCLEX could result in a file descriptor being leaked to a process that should not have access to it. As this patch removes access controls, a policy capability needs to be enabled in policy to always allow these ioctls. Based-on-patch-by: Demi Marie Obenour Signed-off-by: Richard Haines [PM: subject line tweak] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 6 ++++++ security/selinux/include/policycap.h | 1 + security/selinux/include/policycap_names.h | 3 ++- security/selinux/include/security.h | 7 +++++++ 4 files changed, 16 insertions(+), 1 deletion(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 1e69f88eb326..b12e14b2797b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3682,6 +3682,12 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd, CAP_OPT_NONE, true); break; + case FIOCLEX: + case FIONCLEX: + if (!selinux_policycap_ioctl_skip_cloexec()) + error = ioctl_has_perm(cred, file, FILE__IOCTL, (u16) cmd); + break; + /* default case assumes that the command will go * to the file's ioctl() function. */ diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h index 2ec038efbb03..a9e572ca4fd9 100644 --- a/security/selinux/include/policycap.h +++ b/security/selinux/include/policycap.h @@ -11,6 +11,7 @@ enum { POLICYDB_CAPABILITY_CGROUPSECLABEL, POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, + POLICYDB_CAPABILITY_IOCTL_SKIP_CLOEXEC, __POLICYDB_CAPABILITY_MAX }; #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h index b89289f092c9..ebd64afe1def 100644 --- a/security/selinux/include/policycap_names.h +++ b/security/selinux/include/policycap_names.h @@ -12,7 +12,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { "always_check_network", "cgroup_seclabel", "nnp_nosuid_transition", - "genfs_seclabel_symlinks" + "genfs_seclabel_symlinks", + "ioctl_skip_cloexec" }; #endif /* _SELINUX_POLICYCAP_NAMES_H_ */ diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index d91a5672de99..84f42fa8012f 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -219,6 +219,13 @@ static inline bool selinux_policycap_genfs_seclabel_symlinks(void) return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); } +static inline bool selinux_policycap_ioctl_skip_cloexec(void) +{ + struct selinux_state *state = &selinux_state; + + return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_IOCTL_SKIP_CLOEXEC]); +} + struct selinux_policy_convert_data; struct selinux_load_state { -- cgit v1.2.3 From 70868c6b8fd80db585da57a264c50a69af8fd3c3 Mon Sep 17 00:00:00 2001 From: Wan Jiabing Date: Mon, 28 Feb 2022 10:56:41 +0800 Subject: docs: fix 'make htmldocs' warning in SCTP.rst Fix following 'make htmldocs' warnings: ./Documentation/security/SCTP.rst:123: WARNING: Title underline too short. security_sctp_assoc_established() ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./Documentation/security/SCTP.rst:123: WARNING: Title underline too short. security_sctp_assoc_established() ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./Documentation/security/SCTP.rst:273: WARNING: Title underline too short. security_sctp_assoc_established() ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./Documentation/security/SCTP.rst:273: WARNING: Title underline too short. security_sctp_assoc_established() ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Fixes: 5e50f5d4ff31 ("security: add sctp_assoc_established hook") Signed-off-by: Wan Jiabing Reviewed-by: Xin Long Signed-off-by: Paul Moore --- Documentation/security/SCTP.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/security/SCTP.rst b/Documentation/security/SCTP.rst index 406cc68b8808..b73eb764a001 100644 --- a/Documentation/security/SCTP.rst +++ b/Documentation/security/SCTP.rst @@ -120,7 +120,7 @@ calls **sctp_peeloff**\(3). security_sctp_assoc_established() -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Called when a COOKIE ACK is received, and the peer secid will be saved into ``@asoc->peer_secid`` for client:: @@ -270,7 +270,7 @@ sockets sid and peer sid to that contained in the ``@asoc sid`` and security_sctp_assoc_established() -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Called when a COOKIE ACK is received where it sets the connection's peer sid to that in ``@skb``:: -- cgit v1.2.3 From cdbec3ede0b8cb318c36f5cc945b9360329cbd25 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Tue, 1 Mar 2022 12:39:20 -0500 Subject: selinux: shorten the policy capability enum names The SELinux policy capability enum names are rather long and follow the "POLICYDB_CAPABILITY_XXX format". While the "POLICYDB_" prefix is helpful in tying the enums to other SELinux policy constants, macros, etc. there is no reason why we need to spell out "CAPABILITY" completely. Shorten "CAPABILITY" to "CAP" in order to make things a bit shorter and cleaner. Moving forward, the SELinux policy capability enum names should follow the "POLICYDB_CAP_XXX" format. Signed-off-by: Paul Moore --- security/selinux/ima.c | 4 ++-- security/selinux/include/policycap.h | 22 +++++++++++----------- security/selinux/include/policycap_names.h | 2 +- security/selinux/include/security.h | 18 +++++++++--------- security/selinux/selinuxfs.c | 2 +- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/security/selinux/ima.c b/security/selinux/ima.c index 727c4e43219d..fded01981a39 100644 --- a/security/selinux/ima.c +++ b/security/selinux/ima.c @@ -29,7 +29,7 @@ static char *selinux_ima_collect_state(struct selinux_state *state) buf_len = strlen("initialized=0;enforcing=0;checkreqprot=0;") + 1; len = strlen(on); - for (i = 0; i < __POLICYDB_CAPABILITY_MAX; i++) + for (i = 0; i < __POLICYDB_CAP_MAX; i++) buf_len += strlen(selinux_policycap_names[i]) + len; buf = kzalloc(buf_len, GFP_KERNEL); @@ -54,7 +54,7 @@ static char *selinux_ima_collect_state(struct selinux_state *state) rc = strlcat(buf, checkreqprot_get(state) ? on : off, buf_len); WARN_ON(rc >= buf_len); - for (i = 0; i < __POLICYDB_CAPABILITY_MAX; i++) { + for (i = 0; i < __POLICYDB_CAP_MAX; i++) { rc = strlcat(buf, selinux_policycap_names[i], buf_len); WARN_ON(rc >= buf_len); diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h index a9e572ca4fd9..2680aa21205c 100644 --- a/security/selinux/include/policycap.h +++ b/security/selinux/include/policycap.h @@ -4,18 +4,18 @@ /* Policy capabilities */ enum { - POLICYDB_CAPABILITY_NETPEER, - POLICYDB_CAPABILITY_OPENPERM, - POLICYDB_CAPABILITY_EXTSOCKCLASS, - POLICYDB_CAPABILITY_ALWAYSNETWORK, - POLICYDB_CAPABILITY_CGROUPSECLABEL, - POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION, - POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS, - POLICYDB_CAPABILITY_IOCTL_SKIP_CLOEXEC, - __POLICYDB_CAPABILITY_MAX + POLICYDB_CAP_NETPEER, + POLICYDB_CAP_OPENPERM, + POLICYDB_CAP_EXTSOCKCLASS, + POLICYDB_CAP_ALWAYSNETWORK, + POLICYDB_CAP_CGROUPSECLABEL, + POLICYDB_CAP_NNP_NOSUID_TRANSITION, + POLICYDB_CAP_GENFS_SECLABEL_SYMLINKS, + POLICYDB_CAP_IOCTL_SKIP_CLOEXEC, + __POLICYDB_CAP_MAX }; -#define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1) +#define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1) -extern const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX]; +extern const char *selinux_policycap_names[__POLICYDB_CAP_MAX]; #endif /* _SELINUX_POLICYCAP_H_ */ diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h index ebd64afe1def..100da7d043db 100644 --- a/security/selinux/include/policycap_names.h +++ b/security/selinux/include/policycap_names.h @@ -5,7 +5,7 @@ #include "policycap.h" /* Policy capability names */ -const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = { +const char *selinux_policycap_names[__POLICYDB_CAP_MAX] = { "network_peer_controls", "open_perms", "extended_socket_class", diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 84f42fa8012f..ace4bd13e808 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -96,7 +96,7 @@ struct selinux_state { #endif bool checkreqprot; bool initialized; - bool policycap[__POLICYDB_CAPABILITY_MAX]; + bool policycap[__POLICYDB_CAP_MAX]; struct page *status_page; struct mutex status_lock; @@ -174,56 +174,56 @@ static inline bool selinux_policycap_netpeer(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_NETPEER]); + return READ_ONCE(state->policycap[POLICYDB_CAP_NETPEER]); } static inline bool selinux_policycap_openperm(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_OPENPERM]); + return READ_ONCE(state->policycap[POLICYDB_CAP_OPENPERM]); } static inline bool selinux_policycap_extsockclass(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_EXTSOCKCLASS]); + return READ_ONCE(state->policycap[POLICYDB_CAP_EXTSOCKCLASS]); } static inline bool selinux_policycap_alwaysnetwork(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_ALWAYSNETWORK]); + return READ_ONCE(state->policycap[POLICYDB_CAP_ALWAYSNETWORK]); } static inline bool selinux_policycap_cgroupseclabel(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_CGROUPSECLABEL]); + return READ_ONCE(state->policycap[POLICYDB_CAP_CGROUPSECLABEL]); } static inline bool selinux_policycap_nnp_nosuid_transition(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION]); + return READ_ONCE(state->policycap[POLICYDB_CAP_NNP_NOSUID_TRANSITION]); } static inline bool selinux_policycap_genfs_seclabel_symlinks(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS]); + return READ_ONCE(state->policycap[POLICYDB_CAP_GENFS_SECLABEL_SYMLINKS]); } static inline bool selinux_policycap_ioctl_skip_cloexec(void) { struct selinux_state *state = &selinux_state; - return READ_ONCE(state->policycap[POLICYDB_CAPABILITY_IOCTL_SKIP_CLOEXEC]); + return READ_ONCE(state->policycap[POLICYDB_CAP_IOCTL_SKIP_CLOEXEC]); } struct selinux_policy_convert_data; diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index f2f6203e0fff..097c6d866ec4 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1983,7 +1983,7 @@ static int sel_make_policycap(struct selinux_fs_info *fsi) struct dentry *dentry = NULL; struct inode *inode = NULL; - for (iter = 0; iter <= POLICYDB_CAPABILITY_MAX; iter++) { + for (iter = 0; iter <= POLICYDB_CAP_MAX; iter++) { if (iter < ARRAY_SIZE(selinux_policycap_names)) dentry = d_alloc_name(fsi->policycap_dir, selinux_policycap_names[iter]); -- cgit v1.2.3