From b01c939d5854bbf1acf6109ba7a0f74993a22b19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Wei=C3=9Fschuh?= Date: Wed, 27 Nov 2024 11:09:22 +0100 Subject: [PATCH 01/13] selinux: add generated av_permissions.h to targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit av_permissions.h was not declared as a target and therefore not cleaned up automatically by kbuild. Suggested-by: Masahiro Yamada Link: https://lore.kernel.org/lkml/CAK7LNATUnCPt03BRFSKh1EH=+Sy0Q48wE4ER0BZdJqOb_44L8w@mail.gmail.com/ Signed-off-by: Thomas Weißschuh Reviewed-by: Masahiro Yamada Signed-off-by: Paul Moore --- security/selinux/Makefile | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/security/selinux/Makefile b/security/selinux/Makefile index 86f0575f670d..66e56e9011df 100644 --- a/security/selinux/Makefile +++ b/security/selinux/Makefile @@ -33,11 +33,10 @@ $(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h quiet_cmd_genhdrs = GEN $(addprefix $(obj)/,$(genhdrs)) cmd_genhdrs = $< $(addprefix $(obj)/,$(genhdrs)) -# see the note above, replace the $targets and 'flask.h' rule with the lines -# below: -# targets += $(genhdrs) +targets += $(genhdrs) + +# see the note above, replace the 'flask.h' rule with the line below: # $(addprefix $(obj)/,$(genhdrs)) &: $(obj)/genheaders FORCE -targets += flask.h $(obj)/flask.h: $(obj)/genheaders FORCE $(call if_changed,genhdrs) From c75c7945cd49c05404b00358108084a175a5fb29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 25 Nov 2024 12:06:44 +0100 Subject: [PATCH 02/13] selinux: use native iterator types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use types for iterators equal to the type of the to be compared values. Reported by clang: ../ss/sidtab.c:126:2: warning: comparison of integers of different signs: 'int' and 'unsigned long' 126 | hash_for_each_rcu(sidtab->context_to_sid, i, entry, list) { | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../hashtable.h:139:51: note: expanded from macro 'hash_for_each_rcu' 139 | for (... ; obj == NULL && (bkt) < HASH_SIZE(name);\ | ~~~ ^ ~~~~~~~~~~~~~~~ ../selinuxfs.c:1520:23: warning: comparison of integers of different signs: 'int' and 'unsigned int' 1520 | for (cpu = *idx; cpu < nr_cpu_ids; ++cpu) { | ~~~ ^ ~~~~~~~~~~ ../hooks.c:412:16: warning: comparison of integers of different signs: 'int' and 'unsigned long' 412 | for (i = 0; i < ARRAY_SIZE(tokens); i++) { | ~ ^ ~~~~~~~~~~~~~~~~~~ Signed-off-by: Christian Göttsche [PM: munged the clang output due to line length concerns] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- security/selinux/selinuxfs.c | 2 +- security/selinux/ss/sidtab.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index f5a08f94e094..2afc45f355a4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -407,7 +407,7 @@ static const struct { static int match_opt_prefix(char *s, int l, char **arg) { - int i; + unsigned int i; for (i = 0; i < ARRAY_SIZE(tokens); i++) { size_t len = tokens[i].len; diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 6cd5bb0ba380..1efb9a57d181 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1515,7 +1515,7 @@ static const struct file_operations sel_avc_hash_stats_ops = { #ifdef CONFIG_SECURITY_SELINUX_AVC_STATS static struct avc_cache_stats *sel_avc_get_stat_idx(loff_t *idx) { - int cpu; + loff_t cpu; for (cpu = *idx; cpu < nr_cpu_ids; ++cpu) { if (!cpu_possible(cpu)) diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index c8848cbba81f..cb7125cc7f8e 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -114,12 +114,12 @@ int sidtab_set_initial(struct sidtab *s, u32 sid, struct context *context) int sidtab_hash_stats(struct sidtab *sidtab, char *page) { - int i; + unsigned int i; int chain_len = 0; int slots_used = 0; int entries = 0; int max_chain_len = 0; - int cur_bucket = 0; + unsigned int cur_bucket = 0; struct sidtab_entry *entry; rcu_read_lock(); From 034294fbfdf0ded4f931f9503d2ca5bbf8b9aebd Mon Sep 17 00:00:00 2001 From: Mikhail Ivanov Date: Tue, 12 Nov 2024 22:52:03 +0800 Subject: [PATCH 03/13] selinux: Fix SCTP error inconsistency in selinux_socket_bind() Check sk->sk_protocol instead of security class to recognize SCTP socket. SCTP socket is initialized with SECCLASS_SOCKET class if policy does not support EXTSOCKCLASS capability. In this case bind(2) hook wrongfully return EAFNOSUPPORT instead of EINVAL. The inconsistency was detected with help of Landlock tests: https://lore.kernel.org/all/b58680ca-81b2-7222-7287-0ac7f4227c3c@huawei-partners.com/ Fixes: 0f8db8cc73df ("selinux: add AF_UNSPEC and INADDR_ANY checks to selinux_socket_bind()") Signed-off-by: Mikhail Ivanov 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 2afc45f355a4..5e5f3398f39d 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -4835,7 +4835,7 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in return err; err_af: /* Note that SCTP services expect -EINVAL, others -EAFNOSUPPORT. */ - if (sksec->sclass == SECCLASS_SCTP_SOCKET) + if (sk->sk_protocol == IPPROTO_SCTP) return -EINVAL; return -EAFNOSUPPORT; } From 4aa176193475d37441cc52b84088542f3a59899a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Wed, 23 Oct 2024 17:27:10 +0200 Subject: [PATCH 04/13] selinux: add support for xperms in conditional policies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for extended permission rules in conditional policies. Currently the kernel accepts such rules already, but evaluating a security decision will hit a BUG() in services_compute_xperms_decision(). Thus reject extended permission rules in conditional policies for current policy versions. Add a new policy version for this feature. Signed-off-by: Christian Göttsche Acked-by: Stephen Smalley Tested-by: Stephen Smalley Signed-off-by: Paul Moore --- security/selinux/include/security.h | 3 ++- security/selinux/ss/avtab.c | 11 +++++++++-- security/selinux/ss/avtab.h | 2 +- security/selinux/ss/conditional.c | 2 +- security/selinux/ss/policydb.c | 5 +++++ security/selinux/ss/services.c | 12 ++++++++---- 6 files changed, 26 insertions(+), 9 deletions(-) diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index c7f2731abd03..10949df22fa4 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -46,10 +46,11 @@ #define POLICYDB_VERSION_INFINIBAND 31 #define POLICYDB_VERSION_GLBLUB 32 #define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */ +#define POLICYDB_VERSION_COND_XPERMS 34 /* extended permissions in conditional policies */ /* Range of policy versions we understand*/ #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COND_XPERMS /* Mask for just the mount related flags */ #define SE_MNTMASK 0x0f diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index 8e400dd736b7..83add633f92a 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -339,7 +339,7 @@ static const uint16_t spec_order[] = { int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol, int (*insertf)(struct avtab *a, const struct avtab_key *k, const struct avtab_datum *d, void *p), - void *p) + void *p, bool conditional) { __le16 buf16[4]; u16 enabled; @@ -457,6 +457,13 @@ int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol, "was specified\n", vers); return -EINVAL; + } else if ((vers < POLICYDB_VERSION_COND_XPERMS) && + (key.specified & AVTAB_XPERMS) && conditional) { + pr_err("SELinux: avtab: policy version %u does not " + "support extended permissions rules in conditional " + "policies and one was specified\n", + vers); + return -EINVAL; } else if (key.specified & AVTAB_XPERMS) { memset(&xperms, 0, sizeof(struct avtab_extended_perms)); rc = next_entry(&xperms.specified, fp, sizeof(u8)); @@ -523,7 +530,7 @@ int avtab_read(struct avtab *a, void *fp, struct policydb *pol) goto bad; for (i = 0; i < nel; i++) { - rc = avtab_read_item(a, fp, pol, avtab_insertf, NULL); + rc = avtab_read_item(a, fp, pol, avtab_insertf, NULL, false); if (rc) { if (rc == -ENOMEM) pr_err("SELinux: avtab: out of memory\n"); diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index f4407185401c..a7cbb80a11eb 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -108,7 +108,7 @@ struct policydb; int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol, int (*insert)(struct avtab *a, const struct avtab_key *k, const struct avtab_datum *d, void *p), - void *p); + void *p, bool conditional); int avtab_read(struct avtab *a, void *fp, struct policydb *pol); int avtab_write_item(struct policydb *p, const struct avtab_node *cur, diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index 64ba95e40a6f..c9a3060f08a4 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -349,7 +349,7 @@ static int cond_read_av_list(struct policydb *p, void *fp, for (i = 0; i < len; i++) { data.dst = &list->nodes[i]; rc = avtab_read_item(&p->te_cond_avtab, fp, p, cond_insertf, - &data); + &data, true); if (rc) { kfree(list->nodes); list->nodes = NULL; diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 383f3ae82a73..3ba5506a3fff 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -155,6 +155,11 @@ static const struct policydb_compat_info policydb_compat[] = { .sym_num = SYM_NUM, .ocon_num = OCON_NUM, }, + { + .version = POLICYDB_VERSION_COND_XPERMS, + .sym_num = SYM_NUM, + .ocon_num = OCON_NUM, + }, }; static const struct policydb_compat_info * diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 971c45d576ba..55fdc7ca232b 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -946,7 +946,7 @@ static void avd_init(struct selinux_policy *policy, struct av_decision *avd) } static void update_xperms_extended_data(u8 specified, - struct extended_perms_data *from, + const struct extended_perms_data *from, struct extended_perms_data *xp_data) { unsigned int i; @@ -967,6 +967,8 @@ static void update_xperms_extended_data(u8 specified, void services_compute_xperms_decision(struct extended_perms_decision *xpermd, struct avtab_node *node) { + u16 specified; + switch (node->datum.u.xperms->specified) { case AVTAB_XPERMS_IOCTLFUNCTION: case AVTAB_XPERMS_NLMSG: @@ -982,17 +984,19 @@ void services_compute_xperms_decision(struct extended_perms_decision *xpermd, BUG(); } - if (node->key.specified == AVTAB_XPERMS_ALLOWED) { + specified = node->key.specified & ~(AVTAB_ENABLED | AVTAB_ENABLED_OLD); + + if (specified == AVTAB_XPERMS_ALLOWED) { xpermd->used |= XPERMS_ALLOWED; update_xperms_extended_data(node->datum.u.xperms->specified, &node->datum.u.xperms->perms, xpermd->allowed); - } else if (node->key.specified == AVTAB_XPERMS_AUDITALLOW) { + } else if (specified == AVTAB_XPERMS_AUDITALLOW) { xpermd->used |= XPERMS_AUDITALLOW; update_xperms_extended_data(node->datum.u.xperms->specified, &node->datum.u.xperms->perms, xpermd->auditallow); - } else if (node->key.specified == AVTAB_XPERMS_DONTAUDIT) { + } else if (specified == AVTAB_XPERMS_DONTAUDIT) { xpermd->used |= XPERMS_DONTAUDIT; update_xperms_extended_data(node->datum.u.xperms->specified, &node->datum.u.xperms->perms, From 2ef6fc99e0d922a54073e7b6d6465c62f4d3b62b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thi=C3=A9baud=20Weksteen?= Date: Thu, 5 Dec 2024 12:21:00 +1100 Subject: [PATCH 05/13] selinux: add netlink nlmsg_type audit message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new audit message type to capture nlmsg-related information. This is similar to LSM_AUDIT_DATA_IOCTL_OP which was added for the other SELinux extended permission (ioctl). Adding a new type is preferred to adding to the existing lsm_network_audit structure which contains irrelevant information for the netlink sockets (i.e., dport, sport). Signed-off-by: Thiébaud Weksteen [PM: change "nlnk-msgtype" to "nl-msgtype" as discussed] Signed-off-by: Paul Moore --- include/linux/lsm_audit.h | 2 ++ security/lsm_audit.c | 3 +++ security/selinux/hooks.c | 4 ++-- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h index 97a8b21eb033..69d2b7bc00ed 100644 --- a/include/linux/lsm_audit.h +++ b/include/linux/lsm_audit.h @@ -77,6 +77,7 @@ struct common_audit_data { #define LSM_AUDIT_DATA_LOCKDOWN 15 #define LSM_AUDIT_DATA_NOTIFICATION 16 #define LSM_AUDIT_DATA_ANONINODE 17 +#define LSM_AUDIT_DATA_NLMSGTYPE 18 union { struct path path; struct dentry *dentry; @@ -98,6 +99,7 @@ struct common_audit_data { struct lsm_ibendport_audit *ibendport; int reason; const char *anonclass; + u16 nlmsg_type; } u; /* this union contains LSM specific data */ union { diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 9a8352972086..b2f565c0990a 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -425,6 +425,9 @@ static void dump_common_audit_data(struct audit_buffer *ab, case LSM_AUDIT_DATA_ANONINODE: audit_log_format(ab, " anonclass=%s", a->u.anonclass); break; + case LSM_AUDIT_DATA_NLMSGTYPE: + audit_log_format(ab, " nl-msgtype=%hu", a->u.nlmsg_type); + break; } /* switch (a->type) */ } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 5e5f3398f39d..617f54abb640 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5939,14 +5939,14 @@ static int nlmsg_sock_has_extended_perms(struct sock *sk, u32 perms, u16 nlmsg_t { struct sk_security_struct *sksec = sk->sk_security; struct common_audit_data ad; - struct lsm_network_audit net; u8 driver; u8 xperm; if (sock_skip_has_perm(sksec->sid)) return 0; - ad_net_init_from_sk(&ad, &net, sk); + ad.type = LSM_AUDIT_DATA_NLMSGTYPE; + ad.u.nlmsg_type = nlmsg_type; driver = nlmsg_type >> 8; xperm = nlmsg_type & 0xff; From 9d8d094fa307a93674d9126bd05adbda8b3c0011 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:39:59 +0100 Subject: [PATCH 06/13] selinux: supply missing field initializers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Please clang by supplying the missing field initializers in the secclass_map variable and sel_fill_super() function. Signed-off-by: Christian Göttsche [PM: tweak subj and commit description] Signed-off-by: Paul Moore --- security/selinux/include/classmap.h | 2 +- security/selinux/selinuxfs.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 2bc20135324a..03e82477dce9 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -179,7 +179,7 @@ const struct security_class_mapping secclass_map[] = { { "anon_inode", { COMMON_FILE_PERMS, NULL } }, { "io_uring", { "override_creds", "sqpoll", "cmd", NULL } }, { "user_namespace", { "create", NULL } }, - { NULL } + /* last one */ { NULL, {} } }; #ifdef __KERNEL__ /* avoid this check when building host programs */ diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 1efb9a57d181..47480eb2189b 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -2001,7 +2001,7 @@ static int sel_fill_super(struct super_block *sb, struct fs_context *fc) [SEL_POLICY] = {"policy", &sel_policy_ops, S_IRUGO}, [SEL_VALIDATE_TRANS] = {"validatetrans", &sel_transition_ops, S_IWUGO}, - /* last one */ {""} + /* last one */ {"", NULL, 0} }; ret = selinux_fs_info_create(sb); From 046b85a993a19c992da317b2c19e168d1da795af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:40:00 +0100 Subject: [PATCH 07/13] selinux: avoid using types indicating user space interaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integer types starting with a double underscore, like __u32, are intended for usage of variables interacting with user-space. Just use the plain variant. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- security/selinux/ss/policydb.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 617f54abb640..7b2e2c60f0f4 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3135,7 +3135,7 @@ static int selinux_inode_setattr(struct mnt_idmap *idmap, struct dentry *dentry, const struct cred *cred = current_cred(); struct inode *inode = d_backing_inode(dentry); unsigned int ia_valid = iattr->ia_valid; - __u32 av = FILE__WRITE; + u32 av = FILE__WRITE; /* ATTR_FORCE is just used for ATTR_KILL_S[UG]ID. */ if (ia_valid & ATTR_FORCE) { diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index 4bba386264a3..5c11069121d3 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -144,7 +144,7 @@ struct range_trans { /* Boolean data type */ struct cond_bool_datum { - __u32 value; /* internal type value */ + u32 value; /* internal type value */ int state; }; From 90903085101107b06158d2407bb2a6045af6dead Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:40:01 +0100 Subject: [PATCH 08/13] selinux: constify and reconcile function parameter names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align the parameter names between declarations and definitions, and constify read-only parameters. Signed-off-by: Christian Göttsche [PM: tweak the subject line] Signed-off-by: Paul Moore --- security/selinux/include/conditional.h | 2 +- security/selinux/include/security.h | 4 ++-- security/selinux/ss/avtab.h | 2 +- security/selinux/ss/services.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/security/selinux/include/conditional.h b/security/selinux/include/conditional.h index 5910bb7c2eca..060833e2dba2 100644 --- a/security/selinux/include/conditional.h +++ b/security/selinux/include/conditional.h @@ -16,7 +16,7 @@ int security_get_bools(struct selinux_policy *policy, u32 *len, char ***names, int **values); -int security_set_bools(u32 len, int *values); +int security_set_bools(u32 len, const int *values); int security_get_bool_value(u32 index); diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 10949df22fa4..1d47850fff45 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -290,7 +290,7 @@ int security_context_to_sid_default(const char *scontext, u32 scontext_len, int security_context_to_sid_force(const char *scontext, u32 scontext_len, u32 *sid); -int security_get_user_sids(u32 callsid, char *username, u32 **sids, u32 *nel); +int security_get_user_sids(u32 fromsid, const char *username, u32 **sids, u32 *nel); int security_port_sid(u8 protocol, u16 port, u32 *out_sid); @@ -308,7 +308,7 @@ int security_validate_transition(u32 oldsid, u32 newsid, u32 tasksid, int security_validate_transition_user(u32 oldsid, u32 newsid, u32 tasksid, u16 tclass); -int security_bounded_transition(u32 oldsid, u32 newsid); +int security_bounded_transition(u32 old_sid, u32 new_sid); int security_sid_mls_copy(u32 sid, u32 mls_sid, u32 *new_sid); diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index a7cbb80a11eb..32b8335cf93e 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -89,7 +89,7 @@ struct avtab { }; void avtab_init(struct avtab *h); -int avtab_alloc(struct avtab *, u32); +int avtab_alloc(struct avtab *h, u32 nrules); int avtab_alloc_dup(struct avtab *new, const struct avtab *orig); void avtab_destroy(struct avtab *h); diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 55fdc7ca232b..1c4ac392df2a 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2712,7 +2712,7 @@ int security_node_sid(u16 domain, */ int security_get_user_sids(u32 fromsid, - char *username, + const char *username, u32 **sids, u32 *nel) { @@ -3034,7 +3034,7 @@ int security_get_bools(struct selinux_policy *policy, } -int security_set_bools(u32 len, int *values) +int security_set_bools(u32 len, const int *values) { struct selinux_state *state = &selinux_state; struct selinux_policy *newpolicy, *oldpolicy; From 5e99b81f48cd565a5341c921e62fd09184d6bb72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:40:02 +0100 Subject: [PATCH 09/13] selinux: rework match_ipv6_addrmask() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constify parameters, add size hints, and simplify control flow. According to godbolt the same assembly is generated. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 1c4ac392df2a..9bd14256a154 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -2597,17 +2597,15 @@ int security_netif_sid(char *name, u32 *if_sid) return rc; } -static int match_ipv6_addrmask(u32 *input, u32 *addr, u32 *mask) +static bool match_ipv6_addrmask(const u32 input[4], const u32 addr[4], const u32 mask[4]) { - int i, fail = 0; + int i; for (i = 0; i < 4; i++) - if (addr[i] != (input[i] & mask[i])) { - fail = 1; - break; - } + if (addr[i] != (input[i] & mask[i])) + return false; - return !fail; + return true; } /** From 83e7e18eed6e06cd1ce82fa4b9c9a05c24f7a80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:40:04 +0100 Subject: [PATCH 10/13] selinux: rename comparison functions for clarity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The functions context_cmp(), mls_context_cmp() and ebitmap_cmp() are not traditional C style compare functions returning -1, 0, and 1 for less than, equal, and greater than; they only return whether their arguments are equal. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/context.c | 2 +- security/selinux/ss/context.h | 14 +++++++------- security/selinux/ss/ebitmap.c | 8 ++++---- security/selinux/ss/ebitmap.h | 2 +- security/selinux/ss/mls_types.h | 2 +- security/selinux/ss/services.c | 2 +- security/selinux/ss/sidtab.c | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/security/selinux/ss/context.c b/security/selinux/ss/context.c index e39990f494dd..a528b7f76280 100644 --- a/security/selinux/ss/context.c +++ b/security/selinux/ss/context.c @@ -20,7 +20,7 @@ u32 context_compute_hash(const struct context *c) * context struct with only the len & str set (and vice versa) * under a given policy. Since context structs from different * policies should never meet, it is safe to hash valid and - * invalid contexts differently. The context_cmp() function + * invalid contexts differently. The context_equal() function * already operates under the same assumption. */ if (c->len) diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index 7ccab2e6965f..dd3b9b5b588e 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -132,13 +132,13 @@ static inline int mls_context_glblub(struct context *dst, return rc; } -static inline int mls_context_cmp(const struct context *c1, - const struct context *c2) +static inline bool mls_context_equal(const struct context *c1, + const struct context *c2) { return ((c1->range.level[0].sens == c2->range.level[0].sens) && - ebitmap_cmp(&c1->range.level[0].cat, &c2->range.level[0].cat) && + ebitmap_equal(&c1->range.level[0].cat, &c2->range.level[0].cat) && (c1->range.level[1].sens == c2->range.level[1].sens) && - ebitmap_cmp(&c1->range.level[1].cat, &c2->range.level[1].cat)); + ebitmap_equal(&c1->range.level[1].cat, &c2->range.level[1].cat)); } static inline void mls_context_destroy(struct context *c) @@ -188,15 +188,15 @@ static inline void context_destroy(struct context *c) mls_context_destroy(c); } -static inline int context_cmp(const struct context *c1, - const struct context *c2) +static inline bool context_equal(const struct context *c1, + const struct context *c2) { if (c1->len && c2->len) return (c1->len == c2->len && !strcmp(c1->str, c2->str)); if (c1->len || c2->len) return 0; return ((c1->user == c2->user) && (c1->role == c2->role) && - (c1->type == c2->type) && mls_context_cmp(c1, c2)); + (c1->type == c2->type) && mls_context_equal(c1, c2)); } u32 context_compute_hash(const struct context *c); diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 99c01be15115..1cc1e7e711e3 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -25,12 +25,12 @@ static struct kmem_cache *ebitmap_node_cachep __ro_after_init; -int ebitmap_cmp(const struct ebitmap *e1, const struct ebitmap *e2) +bool ebitmap_equal(const struct ebitmap *e1, const struct ebitmap *e2) { const struct ebitmap_node *n1, *n2; if (e1->highbit != e2->highbit) - return 0; + return false; n1 = e1->node; n2 = e2->node; @@ -41,9 +41,9 @@ int ebitmap_cmp(const struct ebitmap *e1, const struct ebitmap *e2) } if (n1 || n2) - return 0; + return false; - return 1; + return true; } int ebitmap_cpy(struct ebitmap *dst, const struct ebitmap *src) diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index ba2ac3da1153..49eb33de87e0 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -120,7 +120,7 @@ static inline void ebitmap_node_clr_bit(struct ebitmap_node *n, u32 bit) (bit) < ebitmap_length(e); \ (bit) = ebitmap_next_positive(e, &(n), bit)) -int ebitmap_cmp(const struct ebitmap *e1, const struct ebitmap *e2); +bool ebitmap_equal(const struct ebitmap *e1, const struct ebitmap *e2); int ebitmap_cpy(struct ebitmap *dst, const struct ebitmap *src); int ebitmap_and(struct ebitmap *dst, const struct ebitmap *e1, const struct ebitmap *e2); diff --git a/security/selinux/ss/mls_types.h b/security/selinux/ss/mls_types.h index 7ef6e8cb0cf4..51df2ebd1211 100644 --- a/security/selinux/ss/mls_types.h +++ b/security/selinux/ss/mls_types.h @@ -29,7 +29,7 @@ struct mls_range { static inline int mls_level_eq(const struct mls_level *l1, const struct mls_level *l2) { - return ((l1->sens == l2->sens) && ebitmap_cmp(&l1->cat, &l2->cat)); + return ((l1->sens == l2->sens) && ebitmap_equal(&l1->cat, &l2->cat)); } static inline int mls_level_dom(const struct mls_level *l1, diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 9bd14256a154..e5c9b62e59c1 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -3331,7 +3331,7 @@ int security_net_peersid_resolve(u32 nlbl_sid, u32 nlbl_type, __func__, xfrm_sid); goto out; } - rc = (mls_context_cmp(nlbl_ctx, xfrm_ctx) ? 0 : -EACCES); + rc = (mls_context_equal(nlbl_ctx, xfrm_ctx) ? 0 : -EACCES); if (rc) goto out; diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index cb7125cc7f8e..59f8c09158ef 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -66,7 +66,7 @@ static u32 context_to_sid(struct sidtab *s, struct context *context, u32 hash) hash_for_each_possible_rcu(s->context_to_sid, entry, list, hash) { if (entry->hash != hash) continue; - if (context_cmp(&entry->context, context)) { + if (context_equal(&entry->context, context)) { sid = entry->sid; break; } From f07586160fd5492f8d48e7667e7a5d8797aa5090 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:40:05 +0100 Subject: [PATCH 11/13] selinux: use known type instead of void pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Improve type safety and readability by using the known type. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/avtab.c | 8 +-- security/selinux/ss/avtab.h | 9 +-- security/selinux/ss/conditional.c | 12 ++-- security/selinux/ss/conditional.h | 6 +- security/selinux/ss/ebitmap.c | 4 +- security/selinux/ss/ebitmap.h | 5 +- security/selinux/ss/policydb.c | 91 ++++++++++++++++--------------- security/selinux/ss/policydb.h | 16 +++--- 8 files changed, 77 insertions(+), 74 deletions(-) diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index 83add633f92a..c2c31521cace 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -336,7 +336,7 @@ static const uint16_t spec_order[] = { }; /* clang-format on */ -int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol, +int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *pol, int (*insertf)(struct avtab *a, const struct avtab_key *k, const struct avtab_datum *d, void *p), void *p, bool conditional) @@ -507,7 +507,7 @@ static int avtab_insertf(struct avtab *a, const struct avtab_key *k, return avtab_insert(a, k, d); } -int avtab_read(struct avtab *a, void *fp, struct policydb *pol) +int avtab_read(struct avtab *a, struct policy_file *fp, struct policydb *pol) { int rc; __le32 buf[1]; @@ -550,7 +550,7 @@ int avtab_read(struct avtab *a, void *fp, struct policydb *pol) goto out; } -int avtab_write_item(struct policydb *p, const struct avtab_node *cur, void *fp) +int avtab_write_item(struct policydb *p, const struct avtab_node *cur, struct policy_file *fp) { __le16 buf16[4]; __le32 buf32[ARRAY_SIZE(cur->datum.u.xperms->perms.p)]; @@ -586,7 +586,7 @@ int avtab_write_item(struct policydb *p, const struct avtab_node *cur, void *fp) return 0; } -int avtab_write(struct policydb *p, struct avtab *a, void *fp) +int avtab_write(struct policydb *p, struct avtab *a, struct policy_file *fp) { u32 i; int rc = 0; diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index 32b8335cf93e..850b3453f259 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -105,15 +105,16 @@ static inline void avtab_hash_eval(struct avtab *h, const char *tag) #endif struct policydb; -int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol, +struct policy_file; +int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *pol, int (*insert)(struct avtab *a, const struct avtab_key *k, const struct avtab_datum *d, void *p), void *p, bool conditional); -int avtab_read(struct avtab *a, void *fp, struct policydb *pol); +int avtab_read(struct avtab *a, struct policy_file *fp, struct policydb *pol); int avtab_write_item(struct policydb *p, const struct avtab_node *cur, - void *fp); -int avtab_write(struct policydb *p, struct avtab *a, void *fp); + struct policy_file *fp); +int avtab_write(struct policydb *p, struct avtab *a, struct policy_file *fp); struct avtab_node *avtab_insert_nonunique(struct avtab *h, const struct avtab_key *key, diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index c9a3060f08a4..d8dcaf2ca88f 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -206,7 +206,7 @@ static int bool_isvalid(struct cond_bool_datum *b) return 1; } -int cond_read_bool(struct policydb *p, struct symtab *s, void *fp) +int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct cond_bool_datum *booldatum; @@ -323,7 +323,7 @@ static int cond_insertf(struct avtab *a, const struct avtab_key *k, return 0; } -static int cond_read_av_list(struct policydb *p, void *fp, +static int cond_read_av_list(struct policydb *p, struct policy_file *fp, struct cond_av_list *list, struct cond_av_list *other) { @@ -375,7 +375,7 @@ static int expr_node_isvalid(struct policydb *p, struct cond_expr_node *expr) return 1; } -static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp) +static int cond_read_node(struct policydb *p, struct cond_node *node, struct policy_file *fp) { __le32 buf[2]; u32 i, len; @@ -415,7 +415,7 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp) return cond_read_av_list(p, fp, &node->false_list, &node->true_list); } -int cond_read_list(struct policydb *p, void *fp) +int cond_read_list(struct policydb *p, struct policy_file *fp) { __le32 buf[1]; u32 i, len; @@ -453,7 +453,7 @@ int cond_write_bool(void *vkey, void *datum, void *ptr) char *key = vkey; struct cond_bool_datum *booldatum = datum; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; __le32 buf[3]; u32 len; int rc; @@ -536,7 +536,7 @@ static int cond_write_node(struct policydb *p, struct cond_node *node, return 0; } -int cond_write_list(struct policydb *p, void *fp) +int cond_write_list(struct policydb *p, struct policy_file *fp) { u32 i; __le32 buf[1]; diff --git a/security/selinux/ss/conditional.h b/security/selinux/ss/conditional.h index 8827715bad75..468e98ad3ea1 100644 --- a/security/selinux/ss/conditional.h +++ b/security/selinux/ss/conditional.h @@ -68,10 +68,10 @@ int cond_destroy_bool(void *key, void *datum, void *p); int cond_index_bool(void *key, void *datum, void *datap); -int cond_read_bool(struct policydb *p, struct symtab *s, void *fp); -int cond_read_list(struct policydb *p, void *fp); +int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp); +int cond_read_list(struct policydb *p, struct policy_file *fp); int cond_write_bool(void *key, void *datum, void *ptr); -int cond_write_list(struct policydb *p, void *fp); +int cond_write_list(struct policydb *p, struct policy_file *fp); void cond_compute_av(struct avtab *ctab, struct avtab_key *key, struct av_decision *avd, struct extended_perms *xperms); diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c index 1cc1e7e711e3..43bc19e21960 100644 --- a/security/selinux/ss/ebitmap.c +++ b/security/selinux/ss/ebitmap.c @@ -360,7 +360,7 @@ void ebitmap_destroy(struct ebitmap *e) e->node = NULL; } -int ebitmap_read(struct ebitmap *e, void *fp) +int ebitmap_read(struct ebitmap *e, struct policy_file *fp) { struct ebitmap_node *n = NULL; u32 mapunit, count, startbit, index, i; @@ -478,7 +478,7 @@ int ebitmap_read(struct ebitmap *e, void *fp) goto out; } -int ebitmap_write(const struct ebitmap *e, void *fp) +int ebitmap_write(const struct ebitmap *e, struct policy_file *fp) { struct ebitmap_node *n; u32 bit, count, last_bit, last_startbit; diff --git a/security/selinux/ss/ebitmap.h b/security/selinux/ss/ebitmap.h index 49eb33de87e0..c9569998f287 100644 --- a/security/selinux/ss/ebitmap.h +++ b/security/selinux/ss/ebitmap.h @@ -129,8 +129,9 @@ int ebitmap_contains(const struct ebitmap *e1, const struct ebitmap *e2, int ebitmap_get_bit(const struct ebitmap *e, u32 bit); int ebitmap_set_bit(struct ebitmap *e, u32 bit, int value); void ebitmap_destroy(struct ebitmap *e); -int ebitmap_read(struct ebitmap *e, void *fp); -int ebitmap_write(const struct ebitmap *e, void *fp); +struct policy_file; +int ebitmap_read(struct ebitmap *e, struct policy_file *fp); +int ebitmap_write(const struct ebitmap *e, struct policy_file *fp); u32 ebitmap_hash(const struct ebitmap *e, u32 hash); #ifdef CONFIG_NETLABEL diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 3ba5506a3fff..1b9fdda03e91 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -997,7 +997,7 @@ int policydb_context_isvalid(struct policydb *p, struct context *c) * Read a MLS range structure from a policydb binary * representation file. */ -static int mls_read_range_helper(struct mls_range *r, void *fp) +static int mls_read_range_helper(struct mls_range *r, struct policy_file *fp) { __le32 buf[2]; u32 items; @@ -1057,7 +1057,7 @@ static int mls_read_range_helper(struct mls_range *r, void *fp) * from a policydb binary representation file. */ static int context_read_and_validate(struct context *c, struct policydb *p, - void *fp) + struct policy_file *fp) { __le32 buf[3]; int rc; @@ -1095,7 +1095,7 @@ static int context_read_and_validate(struct context *c, struct policydb *p, * binary representation file. */ -static int str_read(char **strp, gfp_t flags, void *fp, u32 len) +static int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len) { int rc; char *str; @@ -1118,7 +1118,7 @@ static int str_read(char **strp, gfp_t flags, void *fp, u32 len) return 0; } -static int perm_read(struct policydb *p, struct symtab *s, void *fp) +static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct perm_datum *perdatum; @@ -1151,7 +1151,7 @@ static int perm_read(struct policydb *p, struct symtab *s, void *fp) return rc; } -static int common_read(struct policydb *p, struct symtab *s, void *fp) +static int common_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct common_datum *comdatum; @@ -1203,7 +1203,7 @@ static void type_set_init(struct type_set *t) ebitmap_init(&t->negset); } -static int type_set_read(struct type_set *t, void *fp) +static int type_set_read(struct type_set *t, struct policy_file *fp) { __le32 buf[1]; int rc; @@ -1222,7 +1222,7 @@ static int type_set_read(struct type_set *t, void *fp) } static int read_cons_helper(struct policydb *p, struct constraint_node **nodep, - u32 ncons, int allowxtarget, void *fp) + u32 ncons, int allowxtarget, struct policy_file *fp) { struct constraint_node *c, *lc; struct constraint_expr *e, *le; @@ -1316,7 +1316,7 @@ static int read_cons_helper(struct policydb *p, struct constraint_node **nodep, return 0; } -static int class_read(struct policydb *p, struct symtab *s, void *fp) +static int class_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct class_datum *cladatum; @@ -1413,7 +1413,7 @@ static int class_read(struct policydb *p, struct symtab *s, void *fp) return rc; } -static int role_read(struct policydb *p, struct symtab *s, void *fp) +static int role_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct role_datum *role; @@ -1470,7 +1470,7 @@ static int role_read(struct policydb *p, struct symtab *s, void *fp) return rc; } -static int type_read(struct policydb *p, struct symtab *s, void *fp) +static int type_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct type_datum *typdatum; @@ -1522,7 +1522,7 @@ static int type_read(struct policydb *p, struct symtab *s, void *fp) * Read a MLS level structure from a policydb binary * representation file. */ -static int mls_read_level(struct mls_level *lp, void *fp) +static int mls_read_level(struct mls_level *lp, struct policy_file *fp) { __le32 buf[1]; int rc; @@ -1544,7 +1544,7 @@ static int mls_read_level(struct mls_level *lp, void *fp) return 0; } -static int user_read(struct policydb *p, struct symtab *s, void *fp) +static int user_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct user_datum *usrdatum; @@ -1595,7 +1595,7 @@ static int user_read(struct policydb *p, struct symtab *s, void *fp) return rc; } -static int sens_read(struct policydb *p, struct symtab *s, void *fp) +static int sens_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct level_datum *levdatum; @@ -1636,7 +1636,7 @@ static int sens_read(struct policydb *p, struct symtab *s, void *fp) return rc; } -static int cat_read(struct policydb *p, struct symtab *s, void *fp) +static int cat_read(struct policydb *p, struct symtab *s, struct policy_file *fp) { char *key = NULL; struct cat_datum *catdatum; @@ -1671,7 +1671,7 @@ static int cat_read(struct policydb *p, struct symtab *s, void *fp) /* clang-format off */ static int (*const read_f[SYM_NUM])(struct policydb *p, struct symtab *s, - void *fp) = { + struct policy_file *fp) = { common_read, class_read, role_read, @@ -1841,7 +1841,7 @@ u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name) return 1U << (perdatum->value - 1); } -static int range_read(struct policydb *p, void *fp) +static int range_read(struct policydb *p, struct policy_file *fp) { struct range_trans *rt = NULL; struct mls_range *r = NULL; @@ -1918,7 +1918,7 @@ static int range_read(struct policydb *p, void *fp) return rc; } -static int filename_trans_read_helper_compat(struct policydb *p, void *fp) +static int filename_trans_read_helper_compat(struct policydb *p, struct policy_file *fp) { struct filename_trans_key key, *ft = NULL; struct filename_trans_datum *last, *datum = NULL; @@ -2003,7 +2003,7 @@ static int filename_trans_read_helper_compat(struct policydb *p, void *fp) return rc; } -static int filename_trans_read_helper(struct policydb *p, void *fp) +static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp) { struct filename_trans_key *ft = NULL; struct filename_trans_datum **dst, *datum, *first = NULL; @@ -2092,7 +2092,7 @@ static int filename_trans_read_helper(struct policydb *p, void *fp) return rc; } -static int filename_trans_read(struct policydb *p, void *fp) +static int filename_trans_read(struct policydb *p, struct policy_file *fp) { u32 nel, i; __le32 buf[1]; @@ -2133,7 +2133,7 @@ static int filename_trans_read(struct policydb *p, void *fp) return 0; } -static int genfs_read(struct policydb *p, void *fp) +static int genfs_read(struct policydb *p, struct policy_file *fp) { int rc; u32 i, j, nel, nel2, len, len2; @@ -2247,7 +2247,7 @@ static int genfs_read(struct policydb *p, void *fp) } static int ocontext_read(struct policydb *p, - const struct policydb_compat_info *info, void *fp) + const struct policydb_compat_info *info, struct policy_file *fp) { int rc; unsigned int i; @@ -2444,7 +2444,7 @@ static int ocontext_read(struct policydb *p, * Read the configuration data from a policy database binary * representation file into a policy database structure. */ -int policydb_read(struct policydb *p, void *fp) +int policydb_read(struct policydb *p, struct policy_file *fp) { struct role_allow *ra, *lra; struct role_trans_key *rtk = NULL; @@ -2767,7 +2767,7 @@ int policydb_read(struct policydb *p, void *fp) * Write a MLS level structure to a policydb binary * representation file. */ -static int mls_write_level(struct mls_level *l, void *fp) +static int mls_write_level(struct mls_level *l, struct policy_file *fp) { __le32 buf[1]; int rc; @@ -2788,7 +2788,7 @@ static int mls_write_level(struct mls_level *l, void *fp) * Write a MLS range structure to a policydb binary * representation file. */ -static int mls_write_range_helper(struct mls_range *r, void *fp) +static int mls_write_range_helper(struct mls_range *r, struct policy_file *fp) { __le32 buf[3]; size_t items; @@ -2828,7 +2828,7 @@ static int sens_write(void *vkey, void *datum, void *ptr) char *key = vkey; struct level_datum *levdatum = datum; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; __le32 buf[2]; size_t len; int rc; @@ -2856,7 +2856,7 @@ static int cat_write(void *vkey, void *datum, void *ptr) char *key = vkey; struct cat_datum *catdatum = datum; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; __le32 buf[3]; size_t len; int rc; @@ -2881,7 +2881,7 @@ static int role_trans_write_one(void *key, void *datum, void *ptr) struct role_trans_key *rtk = key; struct role_trans_datum *rtd = datum; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; struct policydb *p = pd->p; __le32 buf[3]; int rc; @@ -2901,7 +2901,7 @@ static int role_trans_write_one(void *key, void *datum, void *ptr) return 0; } -static int role_trans_write(struct policydb *p, void *fp) +static int role_trans_write(struct policydb *p, struct policy_file *fp) { struct policy_data pd = { .p = p, .fp = fp }; __le32 buf[1]; @@ -2915,7 +2915,7 @@ static int role_trans_write(struct policydb *p, void *fp) return hashtab_map(&p->role_tr, role_trans_write_one, &pd); } -static int role_allow_write(struct role_allow *r, void *fp) +static int role_allow_write(struct role_allow *r, struct policy_file *fp) { struct role_allow *ra; __le32 buf[2]; @@ -2943,7 +2943,7 @@ static int role_allow_write(struct role_allow *r, void *fp) * Write a security context structure * to a policydb binary representation file. */ -static int context_write(struct policydb *p, struct context *c, void *fp) +static int context_write(struct policydb *p, struct context *c, struct policy_file *fp) { int rc; __le32 buf[3]; @@ -2996,7 +2996,7 @@ static int common_write(void *vkey, void *datum, void *ptr) char *key = vkey; struct common_datum *comdatum = datum; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; __le32 buf[4]; size_t len; int rc; @@ -3021,7 +3021,7 @@ static int common_write(void *vkey, void *datum, void *ptr) return 0; } -static int type_set_write(struct type_set *t, void *fp) +static int type_set_write(struct type_set *t, struct policy_file *fp) { int rc; __le32 buf[1]; @@ -3040,7 +3040,7 @@ static int type_set_write(struct type_set *t, void *fp) } static int write_cons_helper(struct policydb *p, struct constraint_node *node, - void *fp) + struct policy_file *fp) { struct constraint_node *c; struct constraint_expr *e; @@ -3091,7 +3091,7 @@ static int class_write(void *vkey, void *datum, void *ptr) char *key = vkey; struct class_datum *cladatum = datum; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; struct policydb *p = pd->p; struct constraint_node *c; __le32 buf[6]; @@ -3176,7 +3176,7 @@ static int role_write(void *vkey, void *datum, void *ptr) char *key = vkey; struct role_datum *role = datum; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; struct policydb *p = pd->p; __le32 buf[3]; size_t items, len; @@ -3216,7 +3216,7 @@ static int type_write(void *vkey, void *datum, void *ptr) struct type_datum *typdatum = datum; struct policy_data *pd = ptr; struct policydb *p = pd->p; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; __le32 buf[4]; int rc; size_t items, len; @@ -3257,7 +3257,7 @@ static int user_write(void *vkey, void *datum, void *ptr) struct user_datum *usrdatum = datum; struct policy_data *pd = ptr; struct policydb *p = pd->p; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; __le32 buf[3]; size_t items, len; int rc; @@ -3306,7 +3306,8 @@ static int (*const write_f[SYM_NUM])(void *key, void *datum, void *datap) = { /* clang-format on */ static int ocontext_write(struct policydb *p, - const struct policydb_compat_info *info, void *fp) + const struct policydb_compat_info *info, + struct policy_file *fp) { unsigned int i, j; int rc; @@ -3442,7 +3443,7 @@ static int ocontext_write(struct policydb *p, return 0; } -static int genfs_write(struct policydb *p, void *fp) +static int genfs_write(struct policydb *p, struct policy_file *fp) { struct genfs *genfs; struct ocontext *c; @@ -3500,7 +3501,7 @@ static int range_write_helper(void *key, void *data, void *ptr) struct range_trans *rt = key; struct mls_range *r = data; struct policy_data *pd = ptr; - void *fp = pd->fp; + struct policy_file *fp = pd->fp; struct policydb *p = pd->p; int rc; @@ -3522,7 +3523,7 @@ static int range_write_helper(void *key, void *data, void *ptr) return 0; } -static int range_write(struct policydb *p, void *fp) +static int range_write(struct policydb *p, struct policy_file *fp) { __le32 buf[1]; int rc; @@ -3549,7 +3550,7 @@ static int filename_write_helper_compat(void *key, void *data, void *ptr) struct filename_trans_key *ft = key; struct filename_trans_datum *datum = data; struct ebitmap_node *node; - void *fp = ptr; + struct policy_file *fp = ptr; __le32 buf[4]; int rc; u32 bit, len = strlen(ft->name); @@ -3586,7 +3587,7 @@ static int filename_write_helper(void *key, void *data, void *ptr) { struct filename_trans_key *ft = key; struct filename_trans_datum *datum; - void *fp = ptr; + struct policy_file *fp = ptr; __le32 buf[3]; int rc; u32 ndatum, len = strlen(ft->name); @@ -3631,7 +3632,7 @@ static int filename_write_helper(void *key, void *data, void *ptr) return 0; } -static int filename_trans_write(struct policydb *p, void *fp) +static int filename_trans_write(struct policydb *p, struct policy_file *fp) { __le32 buf[1]; int rc; @@ -3663,7 +3664,7 @@ static int filename_trans_write(struct policydb *p, void *fp) * structure to a policy database binary representation * file. */ -int policydb_write(struct policydb *p, void *fp) +int policydb_write(struct policydb *p, struct policy_file *fp) { unsigned int num_syms; int rc; diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index 5c11069121d3..c699fa52f59a 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -312,14 +312,19 @@ struct policydb { u32 process_trans_perms; } __randomize_layout; +struct policy_file { + char *data; + size_t len; +}; + extern void policydb_destroy(struct policydb *p); extern int policydb_load_isids(struct policydb *p, struct sidtab *s); extern int policydb_context_isvalid(struct policydb *p, struct context *c); extern int policydb_class_isvalid(struct policydb *p, unsigned int class); extern int policydb_type_isvalid(struct policydb *p, unsigned int type); extern int policydb_role_isvalid(struct policydb *p, unsigned int role); -extern int policydb_read(struct policydb *p, void *fp); -extern int policydb_write(struct policydb *p, void *fp); +extern int policydb_read(struct policydb *p, struct policy_file *fp); +extern int policydb_write(struct policydb *p, struct policy_file *fp); extern struct filename_trans_datum * policydb_filenametr_search(struct policydb *p, struct filename_trans_key *key); @@ -342,14 +347,9 @@ policydb_roletr_search(struct policydb *p, struct role_trans_key *key); #define POLICYDB_MAGIC SELINUX_MAGIC #define POLICYDB_STRING "SE Linux" -struct policy_file { - char *data; - size_t len; -}; - struct policy_data { struct policydb *p; - void *fp; + struct policy_file *fp; }; static inline int next_entry(void *buf, struct policy_file *fp, size_t bytes) From 749153636643aaa793f14e84e864fdaf5ed0620d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:40:06 +0100 Subject: [PATCH 12/13] selinux: avoid unnecessary indirection in struct level_datum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store the owned member of type struct mls_level directly in the parent struct instead of an extra heap allocation. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/mls.c | 6 +++--- security/selinux/ss/policydb.c | 19 ++++++------------- security/selinux/ss/policydb.h | 2 +- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c index 989c809d310d..a6e49269f535 100644 --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -171,7 +171,7 @@ int mls_level_isvalid(struct policydb *p, struct mls_level *l) * levdatum->level->cat and no bit in l->cat is larger than * p->p_cats.nprim. */ - return ebitmap_contains(&levdatum->level->cat, &l->cat, + return ebitmap_contains(&levdatum->level.cat, &l->cat, p->p_cats.nprim); } @@ -289,7 +289,7 @@ int mls_context_to_sid(struct policydb *pol, char oldc, char *scontext, levdatum = symtab_search(&pol->p_levels, sensitivity); if (!levdatum) return -EINVAL; - context->range.level[l].sens = levdatum->level->sens; + context->range.level[l].sens = levdatum->level.sens; /* Extract category set. */ while (next_cat != NULL) { @@ -456,7 +456,7 @@ int mls_convert_context(struct policydb *oldp, struct policydb *newp, if (!levdatum) return -EINVAL; - newc->range.level[l].sens = levdatum->level->sens; + newc->range.level[l].sens = levdatum->level.sens; ebitmap_for_each_positive_bit(&oldc->range.level[l].cat, node, i) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 1b9fdda03e91..0850ea6ae018 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -301,9 +301,7 @@ static int sens_destroy(void *key, void *datum, void *p) kfree(key); if (datum) { levdatum = datum; - if (levdatum->level) - ebitmap_destroy(&levdatum->level->cat); - kfree(levdatum->level); + ebitmap_destroy(&levdatum->level.cat); } kfree(datum); return 0; @@ -635,11 +633,11 @@ static int sens_index(void *key, void *datum, void *datap) p = datap; if (!levdatum->isalias) { - if (!levdatum->level->sens || - levdatum->level->sens > p->p_levels.nprim) + if (!levdatum->level.sens || + levdatum->level.sens > p->p_levels.nprim) return -EINVAL; - p->sym_val_to_name[SYM_LEVELS][levdatum->level->sens - 1] = key; + p->sym_val_to_name[SYM_LEVELS][levdatum->level.sens - 1] = key; } return 0; @@ -1618,12 +1616,7 @@ static int sens_read(struct policydb *p, struct symtab *s, struct policy_file *f if (rc) goto bad; - rc = -ENOMEM; - levdatum->level = kmalloc(sizeof(*levdatum->level), GFP_KERNEL); - if (!levdatum->level) - goto bad; - - rc = mls_read_level(levdatum->level, fp); + rc = mls_read_level(&levdatum->level, fp); if (rc) goto bad; @@ -2844,7 +2837,7 @@ static int sens_write(void *vkey, void *datum, void *ptr) if (rc) return rc; - rc = mls_write_level(levdatum->level, fp); + rc = mls_write_level(&levdatum->level, fp); if (rc) return rc; diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index c699fa52f59a..80d1fa7e4995 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -126,7 +126,7 @@ struct user_datum { /* Sensitivity attributes */ struct level_datum { - struct mls_level *level; /* sensitivity and associated categories */ + struct mls_level level; /* sensitivity and associated categories */ unsigned char isalias; /* is this sensitivity an alias for another? */ }; From 01c2253a0fbdccb58cd79d4ff9ab39964bfb4474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Mon, 16 Dec 2024 17:40:07 +0100 Subject: [PATCH 13/13] selinux: make more use of str_read() when loading the policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify the call sites, and enable future string validation in a single place. Signed-off-by: Christian Göttsche [PM: subject tweak] Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 10 ++-------- security/selinux/ss/policydb.c | 22 ++++++++-------------- security/selinux/ss/policydb.h | 2 ++ 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index d8dcaf2ca88f..1bebfcb9c6a1 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -230,17 +230,11 @@ int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp) goto err; len = le32_to_cpu(buf[2]); - if (((len == 0) || (len == (u32)-1))) - goto err; - rc = -ENOMEM; - key = kmalloc(len + 1, GFP_KERNEL); - if (!key) - goto err; - rc = next_entry(key, fp, len); + rc = str_read(&key, GFP_KERNEL, fp, len); if (rc) goto err; - key[len] = '\0'; + rc = symtab_insert(s, key, booldatum); if (rc) goto err; diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 0850ea6ae018..9ea971943713 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1093,7 +1093,7 @@ static int context_read_and_validate(struct context *c, struct policydb *p, * binary representation file. */ -static int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len) +int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len) { int rc; char *str; @@ -2473,24 +2473,18 @@ int policydb_read(struct policydb *p, struct policy_file *fp) goto bad; } - rc = -ENOMEM; - policydb_str = kmalloc(len + 1, GFP_KERNEL); - if (!policydb_str) { - pr_err("SELinux: unable to allocate memory for policydb " - "string of length %d\n", - len); - goto bad; - } - - rc = next_entry(policydb_str, fp, len); + rc = str_read(&policydb_str, GFP_KERNEL, fp, len); if (rc) { - pr_err("SELinux: truncated policydb string identifier\n"); - kfree(policydb_str); + if (rc == -ENOMEM) { + pr_err("SELinux: unable to allocate memory for policydb string of length %d\n", + len); + } else { + pr_err("SELinux: truncated policydb string identifier\n"); + } goto bad; } rc = -EINVAL; - policydb_str[len] = '\0'; if (strcmp(policydb_str, POLICYDB_STRING)) { pr_err("SELinux: policydb string %s does not match " "my string %s\n", diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index 80d1fa7e4995..25650224b6e7 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -386,6 +386,8 @@ static inline char *sym_name(struct policydb *p, unsigned int sym_num, return p->sym_val_to_name[sym_num][element_nr]; } +extern int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len); + extern u16 string_to_security_class(struct policydb *p, const char *name); extern u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name);