From 6f933aa7dfd0bb84b7935168f1d45abec4f702c1 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 5 May 2023 18:48:35 -0400 Subject: [PATCH 01/45] selinux: more Makefile tweaks A few small tweaks to improve the SELinux Makefile: - Define a new variable, 'genhdrs', to represent both flask.h and av_permissions.h; this should help ensure consistent processing for both generated headers. - Move the 'ccflags-y' variable closer to the top, just after the main 'obj-$(CONFIG_SECURITY_SELINUX)' definition to make it more visible and improve the grouping in the Makefile. - Rework some of the vertical whitespace to improve some of the grouping in the Makefile. Reviewed-by: Ondrej Mosnacek Signed-off-by: Paul Moore --- security/selinux/Makefile | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/security/selinux/Makefile b/security/selinux/Makefile index 0aecf9334ec3..ab8c3093d5fd 100644 --- a/security/selinux/Makefile +++ b/security/selinux/Makefile @@ -5,26 +5,25 @@ obj-$(CONFIG_SECURITY_SELINUX) := selinux.o +ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include + selinux-y := avc.o hooks.o selinuxfs.o netlink.o nlmsgtab.o netif.o \ netnode.o netport.o status.o \ ss/ebitmap.o ss/hashtab.o ss/symtab.o ss/sidtab.o ss/avtab.o \ ss/policydb.o ss/services.o ss/conditional.o ss/mls.o ss/context.o selinux-$(CONFIG_SECURITY_NETWORK_XFRM) += xfrm.o - selinux-$(CONFIG_NETLABEL) += netlabel.o - selinux-$(CONFIG_SECURITY_INFINIBAND) += ibpkey.o - selinux-$(CONFIG_IMA) += ima.o -ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include +genhdrs := flask.h av_permissions.h -$(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h +$(addprefix $(obj)/,$(selinux-y)): $(addprefix $(obj)/,$(genhdrs)) -quiet_cmd_flask = GEN $(obj)/flask.h $(obj)/av_permissions.h - cmd_flask = $< $(obj)/flask.h $(obj)/av_permissions.h +quiet_cmd_genhdrs = GEN $(addprefix $(obj)/,$(genhdrs)) + cmd_genhdrs = $< $(addprefix $(obj)/,$(genhdrs)) -targets += flask.h av_permissions.h -$(obj)/flask.h $(obj)/av_permissions.h &: scripts/selinux/genheaders/genheaders FORCE - $(call if_changed,flask) +targets += $(genhdrs) +$(addprefix $(obj)/,$(genhdrs)) &: scripts/selinux/genheaders/genheaders FORCE + $(call if_changed,genhdrs) From 53f3517ae0870fcb398afbba8ff901d0267772b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 20 Apr 2023 17:04:58 +0200 Subject: [PATCH 02/45] selinux: do not leave dangling pointer behind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In case mls_context_cpy() fails due to OOM set the free'd pointer in context_cpy() to NULL to avoid it potentially being dereferenced or free'd again in future. Freeing a NULL pointer is well-defined and a hard NULL dereference crash is at least not exploitable and should give a workable stack trace. Fixes: 12b29f34558b ("selinux: support deferred mapping of contexts") Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/context.h | 1 + 1 file changed, 1 insertion(+) diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index eda32c3d4c0a..44179977f434 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -167,6 +167,7 @@ static inline int context_cpy(struct context *dst, const struct context *src) rc = mls_context_cpy(dst, src); if (rc) { kfree(dst->str); + dst->str = NULL; return rc; } return 0; From 3d9047a06477ffd630a37e733f2a656e08c6d3aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 20 Apr 2023 17:04:59 +0200 Subject: [PATCH 03/45] selinux: adjust typos in comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by codespell(1) Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- security/selinux/ima.c | 2 +- security/selinux/include/audit.h | 2 +- security/selinux/include/ima.h | 2 +- security/selinux/selinuxfs.c | 2 +- security/selinux/ss/services.c | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 79b4890e9936..c49ce2044a17 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -937,7 +937,7 @@ out: } /* - * NOTE: the caller is resposible for freeing the memory even if on error. + * NOTE: the caller is responsible for freeing the memory even if on error. */ static int selinux_add_opt(int token, const char *s, void **mnt_opts) { diff --git a/security/selinux/ima.c b/security/selinux/ima.c index 7daf59667f59..aa34da9b0aeb 100644 --- a/security/selinux/ima.c +++ b/security/selinux/ima.c @@ -4,7 +4,7 @@ * * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) * - * Measure critical data structures maintainted by SELinux + * Measure critical data structures maintained by SELinux * using IMA subsystem. */ #include diff --git a/security/selinux/include/audit.h b/security/selinux/include/audit.h index 406bceb90c6c..d5495134a5b9 100644 --- a/security/selinux/include/audit.h +++ b/security/selinux/include/audit.h @@ -41,7 +41,7 @@ void selinux_audit_rule_free(void *rule); * selinux_audit_rule_match - determine if a context ID matches a rule. * @sid: the context ID to check * @field: the field this rule refers to - * @op: the operater the rule uses + * @op: the operator the rule uses * @rule: pointer to the audit rule to check against * * Returns 1 if the context id matches the rule, 0 if it does not, and diff --git a/security/selinux/include/ima.h b/security/selinux/include/ima.h index 05e04172c86d..93c05e97eb7f 100644 --- a/security/selinux/include/ima.h +++ b/security/selinux/include/ima.h @@ -4,7 +4,7 @@ * * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com) * - * Measure critical data structures maintainted by SELinux + * Measure critical data structures maintained by SELinux * using IMA subsystem. */ diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 69a583b91fc5..7cc7e2f0272b 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -951,7 +951,7 @@ static ssize_t sel_write_create(struct file *file, char *buf, size_t size) * either whitespace or multibyte characters, they shall be * encoded based on the percentage-encoding rule. * If not encoded, the sscanf logic picks up only left-half - * of the supplied name; splitted by a whitespace unexpectedly. + * of the supplied name; split by a whitespace unexpectedly. */ char *r, *w; int c1, c2; diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index f14d1ffe54c5..9571a447e427 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -583,7 +583,7 @@ static void type_attribute_bounds_av(struct policydb *policydb, /* * flag which drivers have permissions - * only looking for ioctl based extended permssions + * only looking for ioctl based extended permissions */ void services_compute_xperms_drivers( struct extended_perms *xperms, From 757010002b97ad0ed89a3b4499c93d23cae429eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 20 Apr 2023 17:05:00 +0200 Subject: [PATCH 04/45] selinux: avc: drop unused function avc_disable() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit f22f9aaf6c3d ("selinux: remove the runtime disable functionality") the function avc_disable() is no longer used. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/avc.c | 19 ------------------- security/selinux/include/avc.h | 3 --- 2 files changed, 22 deletions(-) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index eaed5c2da02b..6bc65830e1a9 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -1203,22 +1203,3 @@ u32 avc_policy_seqno(void) { return selinux_avc.avc_cache.latest_notif; } - -void avc_disable(void) -{ - /* - * If you are looking at this because you have realized that we are - * not destroying the avc_node_cachep it might be easy to fix, but - * I don't know the memory barrier semantics well enough to know. It's - * possible that some other task dereferenced security_ops when - * it still pointed to selinux operations. If that is the case it's - * possible that it is about to use the avc and is about to need the - * avc_node_cachep. I know I could wrap the security.c security_ops call - * in an rcu_lock, but seriously, it's not worth it. Instead I just flush - * the cache and get that memory back. - */ - if (avc_node_cachep) { - avc_flush(); - /* kmem_cache_destroy(avc_node_cachep); */ - } -} diff --git a/security/selinux/include/avc.h b/security/selinux/include/avc.h index 9301222c8e55..9e055f74daf6 100644 --- a/security/selinux/include/avc.h +++ b/security/selinux/include/avc.h @@ -168,9 +168,6 @@ int avc_get_hash_stats(char *page); unsigned int avc_get_cache_threshold(void); void avc_set_cache_threshold(unsigned int cache_threshold); -/* Attempt to free avc node cache */ -void avc_disable(void); - #ifdef CONFIG_SECURITY_SELINUX_AVC_STATS DECLARE_PER_CPU(struct avc_cache_stats, avc_cache_stats); #endif From aeb060ec71547a3fb7f68b8091538e94c5c5187f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 20 Apr 2023 17:05:01 +0200 Subject: [PATCH 05/45] selinux: drop return at end of void function avc_insert() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 539813e4184a ("selinux: stop returning node from avc_insert()") converted the return value of avc_insert() to void but left the now unnecessary trailing return statement. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/avc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 6bc65830e1a9..1074db66e5ff 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -642,7 +642,6 @@ static void avc_insert(u32 ssid, u32 tsid, u16 tclass, hlist_add_head_rcu(&node->list, head); found: spin_unlock_irqrestore(lock, flag); - return; } /** From 4595ae8c4a10a005a24c0994b710c9575f5b46b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 20 Apr 2023 17:05:02 +0200 Subject: [PATCH 06/45] selinux: retain const qualifier on string literal in avtab_hash_eval() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second parameter `tag` of avtab_hash_eval() is only used for printing. In policydb_index() it is called with a string literal: avtab_hash_eval(&p->te_avtab, "rules"); Signed-off-by: Christian Göttsche [PM: slight formatting tweak in description] Signed-off-by: Paul Moore --- security/selinux/ss/avtab.c | 2 +- security/selinux/ss/avtab.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c index 8480ec6c6e75..6766edc0fe68 100644 --- a/security/selinux/ss/avtab.c +++ b/security/selinux/ss/avtab.c @@ -354,7 +354,7 @@ int avtab_alloc_dup(struct avtab *new, const struct avtab *orig) return avtab_alloc_common(new, orig->nslot); } -void avtab_hash_eval(struct avtab *h, char *tag) +void avtab_hash_eval(struct avtab *h, const char *tag) { int i, chain_len, slots_used, max_chain_len; unsigned long long chain2_len_sum; diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h index d3ebea8d146f..d6742fd9c560 100644 --- a/security/selinux/ss/avtab.h +++ b/security/selinux/ss/avtab.h @@ -92,7 +92,7 @@ int avtab_alloc(struct avtab *, u32); int avtab_alloc_dup(struct avtab *new, const struct avtab *orig); struct avtab_datum *avtab_search(struct avtab *h, const struct avtab_key *k); void avtab_destroy(struct avtab *h); -void avtab_hash_eval(struct avtab *h, char *tag); +void avtab_hash_eval(struct avtab *h, const char *tag); struct policydb; int avtab_read_item(struct avtab *a, void *fp, struct policydb *pol, From 4158cb600006371f4359f6ef23a0f1a7255e8843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 20 Apr 2023 17:05:03 +0200 Subject: [PATCH 07/45] selinux: declare read-only data arrays const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The array of mount tokens in only used in match_opt_prefix() and never modified. The array of symtab names is never modified and only used in the DEBUG_HASHES configuration as output. The array of files for the SElinux filesystem sub-directory `ss` is similar to the other `struct tree_descr` usages only read from to construct the containing entries. Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/hooks.c | 2 +- security/selinux/selinuxfs.c | 2 +- security/selinux/ss/policydb.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index c49ce2044a17..99ded60a6b91 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -357,7 +357,7 @@ enum { }; #define A(s, has_arg) {#s, sizeof(#s) - 1, Opt_##s, has_arg} -static struct { +static const struct { const char *name; int len; int opt; diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c index 7cc7e2f0272b..bad1f6b685fd 100644 --- a/security/selinux/selinuxfs.c +++ b/security/selinux/selinuxfs.c @@ -1649,7 +1649,7 @@ static int sel_make_ss_files(struct dentry *dir) struct super_block *sb = dir->d_sb; struct selinux_fs_info *fsi = sb->s_fs_info; int i; - static struct tree_descr files[] = { + static const struct tree_descr files[] = { { "sidtab_hash_stats", &sel_sidtab_hash_stats_ops, S_IRUGO }, }; diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index adcfb63b3550..97c0074f9312 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -42,7 +42,7 @@ #include "services.h" #ifdef DEBUG_HASHES -static const char *symtab_name[SYM_NUM] = { +static const char *const symtab_name[SYM_NUM] = { "common prefixes", "classes", "roles", From c52df19e3759055cf07d1c0030c46ea958163aa9 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Fri, 5 May 2023 18:49:44 -0400 Subject: [PATCH 08/45] selinux: small cleanups in selinux_audit_rule_init() A few small tweaks to selinux_audit_rule_init(): - Adjust how we use the @rc variable so we are not doing any extra work in the common/success case. - Related to the above, rework the 'out' jump label so that the success and error paths are different, simplifying both. - Cleanup some of the vertical whitespace while we are making the other changes. Signed-off-by: Paul Moore --- security/selinux/ss/services.c | 38 ++++++++++++++++------------------ 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c index 9571a447e427..78946b71c1c1 100644 --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -3541,38 +3541,38 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) tmprule = kzalloc(sizeof(struct selinux_audit_rule), GFP_KERNEL); if (!tmprule) return -ENOMEM; - context_init(&tmprule->au_ctxt); rcu_read_lock(); policy = rcu_dereference(state->policy); policydb = &policy->policydb; - tmprule->au_seqno = policy->latest_granting; - switch (field) { case AUDIT_SUBJ_USER: case AUDIT_OBJ_USER: - rc = -EINVAL; userdatum = symtab_search(&policydb->p_users, rulestr); - if (!userdatum) - goto out; + if (!userdatum) { + rc = -EINVAL; + goto err; + } tmprule->au_ctxt.user = userdatum->value; break; case AUDIT_SUBJ_ROLE: case AUDIT_OBJ_ROLE: - rc = -EINVAL; roledatum = symtab_search(&policydb->p_roles, rulestr); - if (!roledatum) - goto out; + if (!roledatum) { + rc = -EINVAL; + goto err; + } tmprule->au_ctxt.role = roledatum->value; break; case AUDIT_SUBJ_TYPE: case AUDIT_OBJ_TYPE: - rc = -EINVAL; typedatum = symtab_search(&policydb->p_types, rulestr); - if (!typedatum) - goto out; + if (!typedatum) { + rc = -EINVAL; + goto err; + } tmprule->au_ctxt.type = typedatum->value; break; case AUDIT_SUBJ_SEN: @@ -3582,20 +3582,18 @@ int selinux_audit_rule_init(u32 field, u32 op, char *rulestr, void **vrule) rc = mls_from_string(policydb, rulestr, &tmprule->au_ctxt, GFP_ATOMIC); if (rc) - goto out; + goto err; break; } - rc = 0; -out: rcu_read_unlock(); - if (rc) { - selinux_audit_rule_free(tmprule); - tmprule = NULL; - } - *rule = tmprule; + return 0; +err: + rcu_read_unlock(); + selinux_audit_rule_free(tmprule); + *rule = NULL; return rc; } From 48f2c681df4329b50fc92516c10e0398ca127242 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Sat, 1 Apr 2023 14:00:00 +0200 Subject: [PATCH 09/45] pstore/ram: Convert to platform remove callback returning void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The .remove() callback for a platform driver returns an int which makes many driver authors wrongly assume it's possible to do error handling by returning an error code. However the value returned is (mostly) ignored and this typically results in resource leaks. To improve here there is a quest to make the remove callback return void. In the first step of this quest all drivers are converted to .remove_new() which already returns void. Trivially convert this driver from always returning zero in the remove callback to the void returning variant. Signed-off-by: Uwe Kleine-König Reviewed-by: Guilherme G. Piccoli Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230401120000.2487153-1-u.kleine-koenig@pengutronix.de --- fs/pstore/ram.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index ade66dbe5f39..2f625e1fa8d8 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -875,7 +875,7 @@ fail_out: return err; } -static int ramoops_remove(struct platform_device *pdev) +static void ramoops_remove(struct platform_device *pdev) { struct ramoops_context *cxt = &oops_cxt; @@ -885,8 +885,6 @@ static int ramoops_remove(struct platform_device *pdev) cxt->pstore.bufsize = 0; ramoops_free_przs(cxt); - - return 0; } static const struct of_device_id dt_match[] = { @@ -896,7 +894,7 @@ static const struct of_device_id dt_match[] = { static struct platform_driver ramoops_driver = { .probe = ramoops_probe, - .remove = ramoops_remove, + .remove_new = ramoops_remove, .driver = { .name = "ramoops", .of_match_table = dt_match, From 3a3d8fce31a49363cc31880dce5e3b0617c9c38b Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 8 May 2023 19:02:33 +0200 Subject: [PATCH 10/45] smack: Retrieve transmuting information in smack_inode_getsecurity() Enhance smack_inode_getsecurity() to retrieve the value for SMACK64TRANSMUTE from the inode security blob, similarly to SMACK64. This helps to display accurate values in the situation where the security labels come from mount options and not from xattrs. Signed-off-by: Roberto Sassu Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 7a3e9ab137d8..c7e37ed2799a 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1463,10 +1463,19 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap, struct super_block *sbp; struct inode *ip = inode; struct smack_known *isp; + struct inode_smack *ispp; + size_t label_len; + char *label = NULL; - if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) + if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) { isp = smk_of_inode(inode); - else { + } else if (strcmp(name, XATTR_SMACK_TRANSMUTE) == 0) { + ispp = smack_inode(inode); + if (ispp->smk_flags & SMK_INODE_TRANSMUTE) + label = TRANS_TRUE; + else + label = ""; + } else { /* * The rest of the Smack xattrs are only on sockets. */ @@ -1488,13 +1497,18 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap, return -EOPNOTSUPP; } + if (!label) + label = isp->smk_known; + + label_len = strlen(label); + if (alloc) { - *buffer = kstrdup(isp->smk_known, GFP_KERNEL); + *buffer = kstrdup(label, GFP_KERNEL); if (*buffer == NULL) return -ENOMEM; } - return strlen(isp->smk_known); + return label_len; } From 2c085f3a8f23c9b444e8b99d93c15d7ce870fc4e Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 8 May 2023 19:02:34 +0200 Subject: [PATCH 11/45] smack: Record transmuting in smk_transmuted smack_dentry_create_files_as() determines whether transmuting should occur based on the label of the parent directory the new inode will be added to, and not the label of the directory where it is created. This helps for example to do transmuting on overlayfs, since the latter first creates the inode in the working directory, and then moves it to the correct destination. However, despite smack_dentry_create_files_as() provides the correct label, smack_inode_init_security() does not know from passed information whether or not transmuting occurred. Without this information, smack_inode_init_security() cannot set SMK_INODE_CHANGED in smk_flags, which will result in the SMACK64TRANSMUTE xattr not being set in smack_d_instantiate(). Thus, add the smk_transmuted field to the task_smack structure, and set it in smack_dentry_create_files_as() to smk_task if transmuting occurred. If smk_task is equal to smk_transmuted in smack_inode_init_security(), act as if transmuting was successful but without taking the label from the parent directory (the inode label was already set correctly from the current credentials in smack_inode_alloc_security()). Signed-off-by: Roberto Sassu Signed-off-by: Casey Schaufler --- security/smack/smack.h | 1 + security/smack/smack_lsm.c | 41 +++++++++++++++++++++++++++----------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/security/smack/smack.h b/security/smack/smack.h index e2239be7bd60..aa15ff56ed6e 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -120,6 +120,7 @@ struct inode_smack { struct task_smack { struct smack_known *smk_task; /* label for access control */ struct smack_known *smk_forked; /* label when forked */ + struct smack_known *smk_transmuted;/* label when transmuted */ struct list_head smk_rules; /* per task access rules */ struct mutex smk_rules_lock; /* lock for the rules */ struct list_head smk_relabel; /* transit allowed labels */ diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index c7e37ed2799a..6e270cf3fd30 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -933,8 +933,9 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, const char **name, void **value, size_t *len) { + struct task_smack *tsp = smack_cred(current_cred()); struct inode_smack *issp = smack_inode(inode); - struct smack_known *skp = smk_of_current(); + struct smack_known *skp = smk_of_task(tsp); struct smack_known *isp = smk_of_inode(inode); struct smack_known *dsp = smk_of_inode(dir); int may; @@ -943,20 +944,34 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir, *name = XATTR_SMACK_SUFFIX; if (value && len) { - rcu_read_lock(); - may = smk_access_entry(skp->smk_known, dsp->smk_known, - &skp->smk_rules); - rcu_read_unlock(); + /* + * If equal, transmuting already occurred in + * smack_dentry_create_files_as(). No need to check again. + */ + if (tsp->smk_task != tsp->smk_transmuted) { + rcu_read_lock(); + may = smk_access_entry(skp->smk_known, dsp->smk_known, + &skp->smk_rules); + rcu_read_unlock(); + } /* - * If the access rule allows transmutation and - * the directory requests transmutation then - * by all means transmute. + * In addition to having smk_task equal to smk_transmuted, + * if the access rule allows transmutation and the directory + * requests transmutation then by all means transmute. * Mark the inode as changed. */ - if (may > 0 && ((may & MAY_TRANSMUTE) != 0) && - smk_inode_transmutable(dir)) { - isp = dsp; + if ((tsp->smk_task == tsp->smk_transmuted) || + (may > 0 && ((may & MAY_TRANSMUTE) != 0) && + smk_inode_transmutable(dir))) { + /* + * The caller of smack_dentry_create_files_as() + * should have overridden the current cred, so the + * inode label was already set correctly in + * smack_inode_alloc_security(). + */ + if (tsp->smk_task != tsp->smk_transmuted) + isp = dsp; issp->smk_flags |= SMK_INODE_CHANGED; } @@ -4767,8 +4782,10 @@ static int smack_dentry_create_files_as(struct dentry *dentry, int mode, * providing access is transmuting use the containing * directory label instead of the process label. */ - if (may > 0 && (may & MAY_TRANSMUTE)) + if (may > 0 && (may & MAY_TRANSMUTE)) { ntsp->smk_task = isp->smk_inode; + ntsp->smk_transmuted = ntsp->smk_task; + } } return 0; } From 60592fb6b67c653beaa2e7acad9a9d7aa0b71dff Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Fri, 12 May 2023 02:25:28 +0000 Subject: [PATCH 12/45] coredump, vmcore: Set p_align to 4 for PT_NOTE Tools like readelf/llvm-readelf use p_align to parse a PT_NOTE program header as an array of 4-byte entries or 8-byte entries. Currently, there are workarounds[1] in place for Linux to treat p_align==0 as 4. However, it would be more appropriate to set the correct alignment so that tools do not have to rely on guesswork. FreeBSD coredumps set p_align to 4 as well. [1]: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=82ed9683ec099d8205dc499ac84febc975235af6 [2]: https://reviews.llvm.org/D150022 Signed-off-by: Fangrui Song Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230512022528.3430327-1-maskray@google.com --- fs/binfmt_elf.c | 2 +- fs/binfmt_elf_fdpic.c | 2 +- fs/proc/vmcore.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 1033fbdfdbec..44b4c42ab8e8 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1517,7 +1517,7 @@ static void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, loff_t offset) phdr->p_filesz = sz; phdr->p_memsz = 0; phdr->p_flags = 0; - phdr->p_align = 0; + phdr->p_align = 4; } static void fill_note(struct memelfnote *note, const char *name, int type, diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index 05a1471d5283..d76ad3d4f676 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -1269,7 +1269,7 @@ static inline void fill_elf_note_phdr(struct elf_phdr *phdr, int sz, loff_t offs phdr->p_filesz = sz; phdr->p_memsz = 0; phdr->p_flags = 0; - phdr->p_align = 0; + phdr->p_align = 4; return; } diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c index 03f5963914a1..cb80a7703d58 100644 --- a/fs/proc/vmcore.c +++ b/fs/proc/vmcore.c @@ -877,7 +877,7 @@ static int __init merge_note_headers_elf64(char *elfptr, size_t *elfsz, phdr.p_offset = roundup(note_off, PAGE_SIZE); phdr.p_vaddr = phdr.p_paddr = 0; phdr.p_filesz = phdr.p_memsz = phdr_sz; - phdr.p_align = 0; + phdr.p_align = 4; /* Add merged PT_NOTE program header*/ tmp = elfptr + sizeof(Elf64_Ehdr); @@ -1068,7 +1068,7 @@ static int __init merge_note_headers_elf32(char *elfptr, size_t *elfsz, phdr.p_offset = roundup(note_off, PAGE_SIZE); phdr.p_vaddr = phdr.p_paddr = 0; phdr.p_filesz = phdr.p_memsz = phdr_sz; - phdr.p_align = 0; + phdr.p_align = 4; /* Add merged PT_NOTE program header*/ tmp = elfptr + sizeof(Elf32_Ehdr); From e455ca40dbcf2cd50d1e59bf4b2752b300bcdad4 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 17 May 2023 15:10:52 +0200 Subject: [PATCH 13/45] audit: avoid missing-prototype warnings Building with 'make W=1' reveals two function definitions without a previous prototype in the audit code: lib/compat_audit.c:32:5: error: no previous prototype for 'audit_classify_compat_syscall' [-Werror=missing-prototypes] kernel/audit.c:1813:14: error: no previous prototype for 'audit_serial' [-Werror=missing-prototypes] The first one needs a declaration from linux/audit.h but cannot include that header without causing conflicting (compat) syscall number definitions, so move the it into linux/audit_arch.h. The second one is declared conditionally based on CONFIG_AUDITSYSCALL but needed as a local function even when that option is disabled, so move the declaration out of the #ifdef block. Signed-off-by: Arnd Bergmann Signed-off-by: Paul Moore --- include/linux/audit.h | 2 -- include/linux/audit_arch.h | 2 ++ kernel/audit.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/audit.h b/include/linux/audit.h index 31086a72e32a..6a3a9e122bb5 100644 --- a/include/linux/audit.h +++ b/include/linux/audit.h @@ -130,8 +130,6 @@ extern unsigned compat_dir_class[]; extern unsigned compat_chattr_class[]; extern unsigned compat_signal_class[]; -extern int audit_classify_compat_syscall(int abi, unsigned syscall); - /* audit_names->type values */ #define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */ #define AUDIT_TYPE_NORMAL 1 /* a "normal" audit record */ diff --git a/include/linux/audit_arch.h b/include/linux/audit_arch.h index 8fdb1afe251a..0e34d673ef17 100644 --- a/include/linux/audit_arch.h +++ b/include/linux/audit_arch.h @@ -21,4 +21,6 @@ enum auditsc_class_t { AUDITSC_NVALS /* count */ }; +extern int audit_classify_compat_syscall(int abi, unsigned syscall); + #endif diff --git a/kernel/audit.h b/kernel/audit.h index c57b008b9914..94738bce40b2 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -259,8 +259,8 @@ extern struct tty_struct *audit_get_tty(void); extern void audit_put_tty(struct tty_struct *tty); /* audit watch/mark/tree functions */ -#ifdef CONFIG_AUDITSYSCALL extern unsigned int audit_serial(void); +#ifdef CONFIG_AUDITSYSCALL extern int auditsc_get_stamp(struct audit_context *ctx, struct timespec64 *t, unsigned int *serial); From e3d9387f002612093dbeaa272f7930ce5108033f Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Thu, 20 Apr 2023 19:17:13 +0200 Subject: [PATCH 14/45] security, lsm: Introduce security_mptcp_add_subflow() MPTCP can create subflows in kernel context, and later indirectly expose them to user-space, via the owning MPTCP socket. As discussed in the reported link, the above causes unexpected failures for server, MPTCP-enabled applications. Let's introduce a new LSM hook to allow the security module to relabel the subflow according to the owning user-space process, via the MPTCP socket owning the subflow. Note that the new hook requires both the MPTCP socket and the new subflow. This could allow future extensions, e.g. explicitly validating the MPTCP <-> subflow linkage. Link: https://lore.kernel.org/mptcp/CAHC9VhTNh-YwiyTds=P1e3rixEDqbRTFj22bpya=+qJqfcaMfg@mail.gmail.com/ Signed-off-by: Paolo Abeni Acked-by: Matthieu Baerts Signed-off-by: Matthieu Baerts Signed-off-by: Paul Moore --- include/linux/lsm_hook_defs.h | 1 + include/linux/security.h | 6 ++++++ net/mptcp/subflow.c | 6 ++++++ security/security.c | 17 +++++++++++++++++ 4 files changed, 30 insertions(+) diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 6bb55e61e8e8..7308a1a7599b 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -343,6 +343,7 @@ 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) +LSM_HOOK(int, 0, mptcp_add_subflow, struct sock *sk, struct sock *ssk) #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND diff --git a/include/linux/security.h b/include/linux/security.h index e2734e9e44d5..32828502f09e 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1465,6 +1465,7 @@ 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); +int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk); #else /* CONFIG_SECURITY_NETWORK */ static inline int security_unix_stream_connect(struct sock *sock, @@ -1692,6 +1693,11 @@ static inline int security_sctp_assoc_established(struct sctp_association *asoc, { return 0; } + +static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) +{ + return 0; +} #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index ba065b66551a..76952cf74fc0 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -1694,6 +1694,10 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family, lock_sock_nested(sf->sk, SINGLE_DEPTH_NESTING); + err = security_mptcp_add_subflow(sk, sf->sk); + if (err) + goto release_ssk; + /* the newly created socket has to be in the same cgroup as its parent */ mptcp_attach_cgroup(sk, sf->sk); @@ -1706,6 +1710,8 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family, get_net_track(net, &sf->sk->ns_tracker, GFP_KERNEL); sock_inuse_add(net, 1); err = tcp_set_ulp(sf->sk, "mptcp"); + +release_ssk: release_sock(sf->sk); if (err) { diff --git a/security/security.c b/security/security.c index d5ff7ff45b77..0e7be1da0b3d 100644 --- a/security/security.c +++ b/security/security.c @@ -4667,6 +4667,23 @@ int security_sctp_assoc_established(struct sctp_association *asoc, } EXPORT_SYMBOL(security_sctp_assoc_established); +/** + * security_mptcp_add_subflow() - Inherit the LSM label from the MPTCP socket + * @sk: the owning MPTCP socket + * @ssk: the new subflow + * + * Update the labeling for the given MPTCP subflow, to match the one of the + * owning MPTCP socket. This hook has to be called after the socket creation and + * initialization via the security_socket_create() and + * security_socket_post_create() LSM hooks. + * + * Return: Returns 0 on success or a negative error code on failure. + */ +int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk) +{ + return call_int_hook(mptcp_add_subflow, 0, sk, ssk); +} + #endif /* CONFIG_SECURITY_NETWORK */ #ifdef CONFIG_SECURITY_INFINIBAND From 85c3222dddc6697dc7513623983a2ba748ba710e Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Thu, 20 Apr 2023 19:17:14 +0200 Subject: [PATCH 15/45] selinux: Implement mptcp_add_subflow hook Newly added subflows should inherit the LSM label from the associated MPTCP socket regardless of the current context. This patch implements the above copying sid and class from the MPTCP socket context, deleting the existing subflow label, if any, and then re-creating the correct one. The new helper reuses the selinux_netlbl_sk_security_free() function, and the latter can end-up being called multiple times with the same argument; we additionally need to make it idempotent. Signed-off-by: Paolo Abeni Acked-by: Matthieu Baerts Signed-off-by: Matthieu Baerts Signed-off-by: Paul Moore --- security/selinux/hooks.c | 16 ++++++++++++++++ security/selinux/netlabel.c | 8 ++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 99ded60a6b91..03660e551094 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5379,6 +5379,21 @@ static void selinux_sctp_sk_clone(struct sctp_association *asoc, struct sock *sk selinux_netlbl_sctp_sk_clone(sk, newsk); } +static int selinux_mptcp_add_subflow(struct sock *sk, struct sock *ssk) +{ + struct sk_security_struct *ssksec = ssk->sk_security; + struct sk_security_struct *sksec = sk->sk_security; + + ssksec->sclass = sksec->sclass; + ssksec->sid = sksec->sid; + + /* replace the existing subflow label deleting the existing one + * and re-recreating a new label using the updated context + */ + selinux_netlbl_sk_security_free(ssksec); + return selinux_netlbl_socket_post_create(ssk, ssk->sk_family); +} + static int selinux_inet_conn_request(const struct sock *sk, struct sk_buff *skb, struct request_sock *req) { @@ -7074,6 +7089,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { 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(mptcp_add_subflow, selinux_mptcp_add_subflow), 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), diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c index 767c670d33ea..528f5186e912 100644 --- a/security/selinux/netlabel.c +++ b/security/selinux/netlabel.c @@ -154,8 +154,12 @@ void selinux_netlbl_err(struct sk_buff *skb, u16 family, int error, int gateway) */ void selinux_netlbl_sk_security_free(struct sk_security_struct *sksec) { - if (sksec->nlbl_secattr != NULL) - netlbl_secattr_free(sksec->nlbl_secattr); + if (!sksec->nlbl_secattr) + return; + + netlbl_secattr_free(sksec->nlbl_secattr); + sksec->nlbl_secattr = NULL; + sksec->nlbl_state = NLBL_UNSET; } /** From ed99135f7621459ef873991115372ba1afe88a04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 11 May 2023 14:31:47 +0200 Subject: [PATCH 16/45] selinux: keep context struct members in sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 53f3517ae087 ("selinux: do not leave dangling pointer behind") reset the `str` field of the `context` struct in an OOM error branch. In this struct the fields `str` and `len` are coupled and should be kept in sync. Set the length to zero according to the string be set to NULL. Fixes: 53f3517ae087 ("selinux: do not leave dangling pointer behind") Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/context.h | 1 + 1 file changed, 1 insertion(+) diff --git a/security/selinux/ss/context.h b/security/selinux/ss/context.h index 44179977f434..aed704b8c642 100644 --- a/security/selinux/ss/context.h +++ b/security/selinux/ss/context.h @@ -168,6 +168,7 @@ static inline int context_cpy(struct context *dst, const struct context *src) if (rc) { kfree(dst->str); dst->str = NULL; + dst->len = 0; return rc; } return 0; From eb14232fb7713fa6258fa876d6d387c03ec21868 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Fri, 12 May 2023 11:21:55 +0200 Subject: [PATCH 17/45] selinux: make header files self-including MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Include all necessary headers in header files to enable third party applications, like LSP servers, to resolve all used symbols. ibpkey.h: include "flask.h" for SECINITSID_UNLABELED initial_sid_to_string.h: include for NULL Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/include/ibpkey.h | 1 + security/selinux/include/initial_sid_to_string.h | 3 +++ 2 files changed, 4 insertions(+) diff --git a/security/selinux/include/ibpkey.h b/security/selinux/include/ibpkey.h index c992f83b0aae..875b055849e1 100644 --- a/security/selinux/include/ibpkey.h +++ b/security/selinux/include/ibpkey.h @@ -15,6 +15,7 @@ #define _SELINUX_IB_PKEY_H #include +#include "flask.h" #ifdef CONFIG_SECURITY_INFINIBAND void sel_ib_pkey_flush(void); diff --git a/security/selinux/include/initial_sid_to_string.h b/security/selinux/include/initial_sid_to_string.h index 60820517aa43..ecc6e74fa09b 100644 --- a/security/selinux/include/initial_sid_to_string.h +++ b/security/selinux/include/initial_sid_to_string.h @@ -1,4 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ + +#include + static const char *const initial_sid_to_string[] = { NULL, "kernel", From a495108ea99c64ce6b5727cb163162ba28e27bff Mon Sep 17 00:00:00 2001 From: Gaosheng Cui Date: Thu, 11 May 2023 09:27:15 +0800 Subject: [PATCH 18/45] capability: fix kernel-doc warnings in capability.c Fix all kernel-doc warnings in capability.c: kernel/capability.c:477: warning: Function parameter or member 'idmap' not described in 'privileged_wrt_inode_uidgid' kernel/capability.c:493: warning: Function parameter or member 'idmap' not described in 'capable_wrt_inode_uidgid' Signed-off-by: Gaosheng Cui Acked-by: Serge Hallyn Signed-off-by: Paul Moore --- kernel/capability.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kernel/capability.c b/kernel/capability.c index 3e058f41df32..1a2795102ae4 100644 --- a/kernel/capability.c +++ b/kernel/capability.c @@ -467,6 +467,7 @@ EXPORT_SYMBOL(file_ns_capable); /** * privileged_wrt_inode_uidgid - Do capabilities in the namespace work over the inode? * @ns: The user namespace in question + * @idmap: idmap of the mount @inode was found from * @inode: The inode in question * * Return true if the inode uid and gid are within the namespace. @@ -481,6 +482,7 @@ bool privileged_wrt_inode_uidgid(struct user_namespace *ns, /** * capable_wrt_inode_uidgid - Check nsown_capable and uid and gid mapped + * @idmap: idmap of the mount @inode was found from * @inode: The inode in question * @cap: The capability in question * From 8bfbd046a3a8883c476eab1de6eb526f62bdc9d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Thu, 11 May 2023 14:32:12 +0200 Subject: [PATCH 19/45] selinux: deprecated fs ocon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The object context type `fs`, not to be confused with the well used object context type `fscon`, was introduced in the initial git commit 1da177e4c3f4 ("Linux-2.6.12-rc2") but never actually used since. The paper "A Security Policy Configuration for the Security-Enhanced Linux" [1] mentions it under `7.2 File System Contexts` but also states: Currently, this configuration is unused. The policy statement defining such object contexts is `fscon`, e.g.: fscon 2 3 gen_context(system_u:object_r:conA_t,s0) \ gen_context(system_u:object_r:conB_t,s0) It is not documented at selinuxproject.org or in the SELinux notebook and not supported by the Reference Policy buildsystem - the statement is not properly sorted - and thus not used in the Reference or Fedora Policy. Print a warning message at policy load for each such object context: SELinux: void and deprecated fs ocon 02:03 This topic was initially highlighted by Nicolas Iooss [2]. [1]: https://media.defense.gov/2021/Jul/29/2002815735/-1/-1/0/SELINUX-SECURITY-POLICY-CONFIGURATION-REPORT.PDF [2]: https://lore.kernel.org/selinux/CAJfZ7=mP2eJaq2BfO3y0VnwUJaY2cS2p=HZMN71z1pKjzaT0Eg@mail.gmail.com/ Signed-off-by: Christian Göttsche [PM: tweaked deprecation comment, description line wrapping] Signed-off-by: Paul Moore --- security/selinux/ss/policydb.c | 4 ++++ security/selinux/ss/policydb.h | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 97c0074f9312..31b08b34c722 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2257,6 +2257,10 @@ static int ocontext_read(struct policydb *p, const struct policydb_compat_info * if (rc) goto out; + if (i == OCON_FS) + pr_warn("SELinux: void and deprecated fs ocon %s\n", + c->u.name); + rc = context_read_and_validate(&c->context[0], p, fp); if (rc) goto out; diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index ffc4e7bad205..74b63ed1173f 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -225,7 +225,7 @@ struct genfs { /* object context array indices */ #define OCON_ISID 0 /* initial SIDs */ -#define OCON_FS 1 /* unlabeled file systems */ +#define OCON_FS 1 /* unlabeled file systems (deprecated) */ #define OCON_PORT 2 /* TCP and UDP port numbers */ #define OCON_NETIF 3 /* network interfaces */ #define OCON_NODE 4 /* nodes */ From db1d1e8b9867aae5c3e61ad7859abfcc4a6fd6c7 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 17 Apr 2023 12:55:51 -0400 Subject: [PATCH 20/45] IMA: use vfs_getattr_nosec to get the i_version IMA currently accesses the i_version out of the inode directly when it does a measurement. This is fine for most simple filesystems, but can be problematic with more complex setups (e.g. overlayfs). Make IMA instead call vfs_getattr_nosec to get this info. This allows the filesystem to determine whether and how to report the i_version, and should allow IMA to work properly with a broader class of filesystems in the future. Reported-and-Tested-by: Stefan Berger Reviewed-by: Christian Brauner Signed-off-by: Jeff Layton Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 9 ++++++--- security/integrity/ima/ima_main.c | 12 ++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index d3662f4acadc..c45902e72044 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -13,7 +13,6 @@ #include #include #include -#include #include #include "ima.h" @@ -246,10 +245,11 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, struct inode *inode = file_inode(file); const char *filename = file->f_path.dentry->d_name.name; struct ima_max_digest_data hash; + struct kstat stat; int result = 0; int length; void *tmpbuf; - u64 i_version; + u64 i_version = 0; /* * Always collect the modsig, because IMA might have already collected @@ -268,7 +268,10 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, * to an initial measurement/appraisal/audit, but was modified to * assume the file changed. */ - i_version = inode_query_iversion(inode); + result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE, + AT_STATX_SYNC_AS_STAT); + if (!result && (stat.result_mask & STATX_CHANGE_COOKIE)) + i_version = stat.change_cookie; hash.hdr.algo = algo; hash.hdr.length = hash_digest_size[algo]; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index d66a0a36415e..365db0e43d7c 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -24,7 +24,6 @@ #include #include #include -#include #include #include "ima.h" @@ -164,11 +163,16 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint, mutex_lock(&iint->mutex); if (atomic_read(&inode->i_writecount) == 1) { + struct kstat stat; + update = test_and_clear_bit(IMA_UPDATE_XATTR, &iint->atomic_flags); - if (!IS_I_VERSION(inode) || - !inode_eq_iversion(inode, iint->version) || - (iint->flags & IMA_NEW_FILE)) { + if ((iint->flags & IMA_NEW_FILE) || + vfs_getattr_nosec(&file->f_path, &stat, + STATX_CHANGE_COOKIE, + AT_STATX_SYNC_AS_STAT) || + !(stat.result_mask & STATX_CHANGE_COOKIE) || + stat.change_cookie != iint->version) { iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE); iint->measured_pcrs = 0; if (update) From d031f4e8b493df299123fbb4ec13db870584ed28 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Thu, 11 May 2023 23:48:45 +0900 Subject: [PATCH 21/45] reiserfs: Initialize sec->length in reiserfs_security_init(). syzbot is reporting that sec->length is not initialized. Since security_inode_init_security() returns 0 when initxattrs is provided but call_int_hook(inode_init_security) returned -EOPNOTSUPP, control will reach to "if (sec->length && ...) {" without initializing sec->length. Reported-by: syzbot Closes: https://syzkaller.appspot.com/bug?extid=00a3779539a23cbee38c Signed-off-by: Tetsuo Handa Fixes: 52ca4b6435a4 ("reiserfs: Switch to security_inode_init_security()") Signed-off-by: Paul Moore --- fs/reiserfs/xattr_security.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/reiserfs/xattr_security.c b/fs/reiserfs/xattr_security.c index 6e0a099dd788..078dd8cc312f 100644 --- a/fs/reiserfs/xattr_security.c +++ b/fs/reiserfs/xattr_security.c @@ -67,6 +67,7 @@ int reiserfs_security_init(struct inode *dir, struct inode *inode, sec->name = NULL; sec->value = NULL; + sec->length = 0; /* Don't add selinux attributes on xattrs - they'll never get used */ if (IS_PRIVATE(dir)) From 4432b507445acf3f8e09ce253d4ca852c177b625 Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Wed, 24 May 2023 23:19:53 -0400 Subject: [PATCH 22/45] lsm: fix a number of misspellings A random collection of spelling fixes for source files in the LSM layer. Reviewed-by: Casey Schaufler Signed-off-by: Paul Moore --- security/commoncap.c | 20 ++++++++++---------- security/device_cgroup.c | 2 +- security/lsm_audit.c | 2 +- security/security.c | 4 ++-- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/security/commoncap.c b/security/commoncap.c index 0b3fc2f3afe7..ab5742ab4362 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -314,7 +314,7 @@ int cap_inode_need_killpriv(struct dentry *dentry) * the vfsmount must be passed through @idmap. This function will then * take care to map the inode according to @idmap before checking * permissions. On non-idmapped mounts or if permission checking is to be - * performed on the raw inode simply passs @nop_mnt_idmap. + * performed on the raw inode simply pass @nop_mnt_idmap. * * Return: 0 if successful, -ve on error. */ @@ -522,7 +522,7 @@ static bool validheader(size_t size, const struct vfs_cap_data *cap) * the vfsmount must be passed through @idmap. This function will then * take care to map the inode according to @idmap before checking * permissions. On non-idmapped mounts or if permission checking is to be - * performed on the raw inode simply passs @nop_mnt_idmap. + * performed on the raw inode simply pass @nop_mnt_idmap. * * Return: On success, return the new size; on error, return < 0. */ @@ -630,7 +630,7 @@ static inline int bprm_caps_from_vfs_caps(struct cpu_vfs_cap_data *caps, * the vfsmount must be passed through @idmap. This function will then * take care to map the inode according to @idmap before checking * permissions. On non-idmapped mounts or if permission checking is to be - * performed on the raw inode simply passs @nop_mnt_idmap. + * performed on the raw inode simply pass @nop_mnt_idmap. */ int get_vfs_caps_from_disk(struct mnt_idmap *idmap, const struct dentry *dentry, @@ -1133,7 +1133,7 @@ int cap_task_fix_setuid(struct cred *new, const struct cred *old, int flags) break; case LSM_SETID_FS: - /* juggle the capabilties to follow FSUID changes, unless + /* juggle the capabilities to follow FSUID changes, unless * otherwise suppressed * * FIXME - is fsuser used for all CAP_FS_MASK capabilities? @@ -1184,10 +1184,10 @@ static int cap_safe_nice(struct task_struct *p) } /** - * cap_task_setscheduler - Detemine if scheduler policy change is permitted + * cap_task_setscheduler - Determine if scheduler policy change is permitted * @p: The task to affect * - * Detemine if the requested scheduler policy change is permitted for the + * Determine if the requested scheduler policy change is permitted for the * specified task. * * Return: 0 if permission is granted, -ve if denied. @@ -1198,11 +1198,11 @@ int cap_task_setscheduler(struct task_struct *p) } /** - * cap_task_setioprio - Detemine if I/O priority change is permitted + * cap_task_setioprio - Determine if I/O priority change is permitted * @p: The task to affect * @ioprio: The I/O priority to set * - * Detemine if the requested I/O priority change is permitted for the specified + * Determine if the requested I/O priority change is permitted for the specified * task. * * Return: 0 if permission is granted, -ve if denied. @@ -1213,11 +1213,11 @@ int cap_task_setioprio(struct task_struct *p, int ioprio) } /** - * cap_task_setnice - Detemine if task priority change is permitted + * cap_task_setnice - Determine if task priority change is permitted * @p: The task to affect * @nice: The nice value to set * - * Detemine if the requested task priority change is permitted for the + * Determine if the requested task priority change is permitted for the * specified task. * * Return: 0 if permission is granted, -ve if denied. diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 7507d14eacc7..41fca6487ca3 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -421,7 +421,7 @@ static bool verify_new_ex(struct dev_cgroup *dev_cgroup, } else { /* * new exception in the child will add more devices - * that can be acessed, so it can't match any of + * that can be accessed, so it can't match any of * parent's exceptions, even slightly */ match = match_exception_partial(&dev_cgroup->exceptions, diff --git a/security/lsm_audit.c b/security/lsm_audit.c index 368e77ca43c4..849e832719e2 100644 --- a/security/lsm_audit.c +++ b/security/lsm_audit.c @@ -200,7 +200,7 @@ static void dump_common_audit_data(struct audit_buffer *ab, char comm[sizeof(current->comm)]; /* - * To keep stack sizes in check force programers to notice if they + * To keep stack sizes in check force programmers to notice if they * start making this union too large! See struct lsm_network_audit * as an example of how to deal with large data. */ diff --git a/security/security.c b/security/security.c index d5ff7ff45b77..ee4f1cc4902e 100644 --- a/security/security.c +++ b/security/security.c @@ -2491,7 +2491,7 @@ int security_inode_copy_up_xattr(const char *name) /* * The implementation can return 0 (accept the xattr), 1 (discard the * xattr), -EOPNOTSUPP if it does not know anything about the xattr or - * any other error code incase of an error. + * any other error code in case of an error. */ hlist_for_each_entry(hp, &security_hook_heads.inode_copy_up_xattr, list) { @@ -4676,7 +4676,7 @@ EXPORT_SYMBOL(security_sctp_assoc_established); * @subnet_prefix: subnet prefix of the port * @pkey: IB pkey * - * Check permission to access a pkey when modifing a QP. + * Check permission to access a pkey when modifying a QP. * * Return: Returns 0 if permission is granted. */ From 29cd55fe69e37722c797504cffeb9f9e13df1faf Mon Sep 17 00:00:00 2001 From: Xiu Jianfeng Date: Mon, 29 May 2023 21:00:18 +0800 Subject: [PATCH 23/45] selinux: cleanup exit_sel_fs() declaration exit_sel_fs() has been removed since commit f22f9aaf6c3d ("selinux: remove the runtime disable functionality"). Signed-off-by: Xiu Jianfeng Signed-off-by: Paul Moore --- security/selinux/include/security.h | 1 - 1 file changed, 1 deletion(-) diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 8746fafeb778..815838ba7f2a 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -384,7 +384,6 @@ struct selinux_kernel_status { extern void selinux_status_update_setenforce(int enforcing); extern void selinux_status_update_policyload(int seqno); extern void selinux_complete_init(void); -extern void exit_sel_fs(void); extern struct path selinux_null; extern void selnl_notify_setenforce(int val); extern void selnl_notify_policyload(u32 seqno); From cec5fe700799b3f863d25cf883f02e5735598ab5 Mon Sep 17 00:00:00 2001 From: Ondrej Mosnacek Date: Mon, 29 May 2023 16:05:27 +0200 Subject: [PATCH 24/45] selinux: make labeled NFS work when mounted before policy load Currently, when an NFS filesystem that supports passing LSM/SELinux labels is mounted during early boot (before the SELinux policy is loaded), it ends up mounted without the labeling support (i.e. with Fedora policy all files get the generic NFS label system_u:object_r:nfs_t:s0). This is because the information that the NFS mount supports passing labels (communicated to the LSM layer via the kern_flags argument of security_set_mnt_opts()) gets lost and when the policy is loaded the mount is initialized as if the passing is not supported. Fix this by noting the "native labeling" in newsbsec->flags (using a new SE_SBNATIVE flag) on the pre-policy-loaded call of selinux_set_mnt_opts() and then making sure it is respected on the second call from delayed_superblock_init(). Additionally, make inode_doinit_with_dentry() initialize the inode's label from its extended attributes whenever it doesn't find it already intitialized by the filesystem. This is needed to properly initialize pre-existing inodes when delayed_superblock_init() is called. It should not trigger in any other cases (and if it does, it's still better to initialize the correct label instead of leaving the inode unlabeled). Fixes: eb9ae686507b ("SELinux: Add new labeling type native labels") Tested-by: Scott Mayhew Signed-off-by: Ondrej Mosnacek [PM: fixed 'Fixes' tag format] Signed-off-by: Paul Moore --- security/selinux/hooks.c | 58 ++++++++++++++++++++--------- security/selinux/include/security.h | 1 + 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 03660e551094..d06e350fedee 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -605,6 +605,13 @@ static int selinux_set_mnt_opts(struct super_block *sb, u32 defcontext_sid = 0; int rc = 0; + /* + * Specifying internal flags without providing a place to + * place the results is not allowed + */ + if (kern_flags && !set_kern_flags) + return -EINVAL; + mutex_lock(&sbsec->lock); if (!selinux_initialized()) { @@ -612,6 +619,10 @@ static int selinux_set_mnt_opts(struct super_block *sb, /* Defer initialization until selinux_complete_init, after the initial policy is loaded and the security server is ready to handle calls. */ + if (kern_flags & SECURITY_LSM_NATIVE_LABELS) { + sbsec->flags |= SE_SBNATIVE; + *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS; + } goto out; } rc = -EINVAL; @@ -619,12 +630,6 @@ static int selinux_set_mnt_opts(struct super_block *sb, "before the security server is initialized\n"); goto out; } - if (kern_flags && !set_kern_flags) { - /* Specifying internal flags without providing a place to - * place the results is not allowed */ - rc = -EINVAL; - goto out; - } /* * Binary mount data FS will come through this function twice. Once @@ -757,7 +762,17 @@ static int selinux_set_mnt_opts(struct super_block *sb, * sets the label used on all file below the mountpoint, and will set * the superblock context if not already set. */ - if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !context_sid) { + if (sbsec->flags & SE_SBNATIVE) { + /* + * This means we are initializing a superblock that has been + * mounted before the SELinux was initialized and the + * filesystem requested native labeling. We had already + * returned SECURITY_LSM_NATIVE_LABELS in *set_kern_flags + * in the original mount attempt, so now we just need to set + * the SECURITY_FS_USE_NATIVE behavior. + */ + sbsec->behavior = SECURITY_FS_USE_NATIVE; + } else if (kern_flags & SECURITY_LSM_NATIVE_LABELS && !context_sid) { sbsec->behavior = SECURITY_FS_USE_NATIVE; *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS; } @@ -868,13 +883,6 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, int set_context = (oldsbsec->flags & CONTEXT_MNT); int set_rootcontext = (oldsbsec->flags & ROOTCONTEXT_MNT); - /* - * if the parent was able to be mounted it clearly had no special lsm - * mount options. thus we can safely deal with this superblock later - */ - if (!selinux_initialized()) - return 0; - /* * Specifying internal flags without providing a place to * place the results is not allowed. @@ -882,18 +890,31 @@ static int selinux_sb_clone_mnt_opts(const struct super_block *oldsb, if (kern_flags && !set_kern_flags) return -EINVAL; + mutex_lock(&newsbsec->lock); + + /* + * if the parent was able to be mounted it clearly had no special lsm + * mount options. thus we can safely deal with this superblock later + */ + if (!selinux_initialized()) { + if (kern_flags & SECURITY_LSM_NATIVE_LABELS) { + newsbsec->flags |= SE_SBNATIVE; + *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS; + } + goto out; + } + /* how can we clone if the old one wasn't set up?? */ BUG_ON(!(oldsbsec->flags & SE_SBINITIALIZED)); /* if fs is reusing a sb, make sure that the contexts match */ if (newsbsec->flags & SE_SBINITIALIZED) { + mutex_unlock(&newsbsec->lock); if ((kern_flags & SECURITY_LSM_NATIVE_LABELS) && !set_context) *set_kern_flags |= SECURITY_LSM_NATIVE_LABELS; return selinux_cmp_sb_context(oldsb, newsb); } - mutex_lock(&newsbsec->lock); - newsbsec->flags = oldsbsec->flags; newsbsec->sid = oldsbsec->sid; @@ -1394,8 +1415,11 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent spin_unlock(&isec->lock); switch (sbsec->behavior) { + /* + * In case of SECURITY_FS_USE_NATIVE we need to re-fetch the labels + * via xattr when called from delayed_superblock_init(). + */ case SECURITY_FS_USE_NATIVE: - break; case SECURITY_FS_USE_XATTR: if (!(inode->i_opflags & IOP_XATTR)) { sid = sbsec->def_sid; diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h index 815838ba7f2a..3b605f39e040 100644 --- a/security/selinux/include/security.h +++ b/security/selinux/include/security.h @@ -65,6 +65,7 @@ #define SE_SBPROC 0x0200 #define SE_SBGENFS 0x0400 #define SE_SBGENFS_XATTR 0x0800 +#define SE_SBNATIVE 0x1000 #define CONTEXT_STR "context" #define FSCONTEXT_STR "fscontext" From e6302d5a285b41209ada308ced6f9562cf1c616f Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sun, 28 May 2023 18:20:24 +0200 Subject: [PATCH 25/45] binfmt: Use struct_size() Use struct_size() instead of hand-writing it. It is less verbose, more robust and more informative. Signed-off-by: Christophe JAILLET Acked-by: "Eric W. Biederman" Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/53150beae5dc04dac513dba391a2e4ae8696a7f3.1685290790.git.christophe.jaillet@wanadoo.fr --- fs/binfmt_elf_fdpic.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index d76ad3d4f676..237ce388d06d 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -748,7 +748,6 @@ static int elf_fdpic_map_file(struct elf_fdpic_params *params, struct elf32_phdr *phdr; unsigned long load_addr, stop; unsigned nloads, tmp; - size_t size; int loop, ret; /* allocate a load map table */ @@ -760,8 +759,7 @@ static int elf_fdpic_map_file(struct elf_fdpic_params *params, if (nloads == 0) return -ELIBBAD; - size = sizeof(*loadmap) + nloads * sizeof(*seg); - loadmap = kzalloc(size, GFP_KERNEL); + loadmap = kzalloc(struct_size(loadmap, segs, nloads), GFP_KERNEL); if (!loadmap) return -ENOMEM; From 36650a357eac3e525edbd31db4fe45841b05dc3b Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sun, 28 May 2023 18:20:25 +0200 Subject: [PATCH 26/45] binfmt: Slightly simplify elf_fdpic_map_file() There is no point in initializing 'load_addr' and 'seg' here, they are both re-written just before being used below. Doing so, 'load_addr' can be moved in the #ifdef CONFIG_MMU section. Signed-off-by: Christophe JAILLET Acked-by: "Eric W. Biederman" Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/4f5e4096ad7f17716e924b5bd080e5709fc0b84b.1685290790.git.christophe.jaillet@wanadoo.fr --- fs/binfmt_elf_fdpic.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index 237ce388d06d..1c6c5832af86 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -743,11 +743,12 @@ static int elf_fdpic_map_file(struct elf_fdpic_params *params, struct elf32_fdpic_loadmap *loadmap; #ifdef CONFIG_MMU struct elf32_fdpic_loadseg *mseg; + unsigned long load_addr; #endif struct elf32_fdpic_loadseg *seg; struct elf32_phdr *phdr; - unsigned long load_addr, stop; unsigned nloads, tmp; + unsigned long stop; int loop, ret; /* allocate a load map table */ @@ -768,9 +769,6 @@ static int elf_fdpic_map_file(struct elf_fdpic_params *params, loadmap->version = ELF32_FDPIC_LOADMAP_VERSION; loadmap->nsegs = nloads; - load_addr = params->load_addr; - seg = loadmap->segs; - /* map the requested LOADs into the memory space */ switch (params->flags & ELF_FDPIC_FLAG_ARRANGEMENT) { case ELF_FDPIC_FLAG_CONSTDISP: From 9df6a4870dc371136e90330cfbbc51464ee66993 Mon Sep 17 00:00:00 2001 From: Tianjia Zhang Date: Thu, 1 Jun 2023 14:42:44 +0800 Subject: [PATCH 27/45] integrity: Fix possible multiple allocation in integrity_inode_get() When integrity_inode_get() is querying and inserting the cache, there is a conditional race in the concurrent environment. The race condition is the result of not properly implementing "double-checked locking". In this case, it first checks to see if the iint cache record exists before taking the lock, but doesn't check again after taking the integrity_iint_lock. Fixes: bf2276d10ce5 ("ima: allocating iint improvements") Signed-off-by: Tianjia Zhang Cc: Dmitry Kasatkin Cc: # v3.10+ Signed-off-by: Mimi Zohar --- security/integrity/iint.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/security/integrity/iint.c b/security/integrity/iint.c index c73858e8c6d5..a462df827de2 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -43,12 +43,10 @@ static struct integrity_iint_cache *__integrity_iint_find(struct inode *inode) else if (inode > iint->inode) n = n->rb_right; else - break; + return iint; } - if (!n) - return NULL; - return iint; + return NULL; } /* @@ -113,10 +111,15 @@ struct integrity_iint_cache *integrity_inode_get(struct inode *inode) parent = *p; test_iint = rb_entry(parent, struct integrity_iint_cache, rb_node); - if (inode < test_iint->inode) + if (inode < test_iint->inode) { p = &(*p)->rb_left; - else + } else if (inode > test_iint->inode) { p = &(*p)->rb_right; + } else { + write_unlock(&integrity_iint_lock); + kmem_cache_free(iint_cache, iint); + return test_iint; + } } iint->inode = inode; From ec4a491d180b076191e72ef88c240caead1b20fc Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 1 Jun 2023 17:04:02 -0400 Subject: [PATCH 28/45] selinux: fix Makefile for versions of make < v4.3 As noted in the comments of this commit, the current SELinux Makefile requires features found in make v4.3 or later, which is problematic as the Linux Kernel currently only requires make v3.82. This patch fixes the SELinux Makefile so that it works properly on these older versions of make, and adds a couple of comments to the Makefile about how it can be improved once make v4.3 is required by the kernel. Fixes: 6f933aa7dfd0 ("selinux: more Makefile tweaks") Signed-off-by: Paul Moore --- security/selinux/Makefile | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/security/selinux/Makefile b/security/selinux/Makefile index ab8c3093d5fd..836379639058 100644 --- a/security/selinux/Makefile +++ b/security/selinux/Makefile @@ -3,6 +3,11 @@ # Makefile for building the SELinux module as part of the kernel tree. # +# NOTE: There are a number of improvements that can be made to this Makefile +# once the kernel requires make v4.3 or greater; the most important feature +# lacking in older versions of make is support for grouped targets. These +# improvements are noted inline in the Makefile below ... + obj-$(CONFIG_SECURITY_SELINUX) := selinux.o ccflags-y := -I$(srctree)/security/selinux -I$(srctree)/security/selinux/include @@ -19,11 +24,17 @@ selinux-$(CONFIG_IMA) += ima.o genhdrs := flask.h av_permissions.h -$(addprefix $(obj)/,$(selinux-y)): $(addprefix $(obj)/,$(genhdrs)) +# see the note above, replace the dependency rule with the one below: +# $(addprefix $(obj)/,$(selinux-y)): $(addprefix $(obj)/,$(genhdrs)) +$(addprefix $(obj)/,$(selinux-y)): $(obj)/flask.h quiet_cmd_genhdrs = GEN $(addprefix $(obj)/,$(genhdrs)) cmd_genhdrs = $< $(addprefix $(obj)/,$(genhdrs)) -targets += $(genhdrs) -$(addprefix $(obj)/,$(genhdrs)) &: scripts/selinux/genheaders/genheaders FORCE +# see the note above, replace the $targets and 'flask.h' rule with the lines +# below: +# targets += $(genhdrs) +# $(addprefix $(obj)/,$(genhdrs)) &: scripts/selinux/... +targets += flask.h +$(obj)/flask.h: scripts/selinux/genheaders/genheaders FORCE $(call if_changed,genhdrs) From b1de86d4248b273cb12c4cd7d20c08d459519f7d Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 6 Mar 2023 11:40:36 +0100 Subject: [PATCH 29/45] evm: Complete description of evm_inode_setattr() Add the description for missing parameters of evm_inode_setattr() to avoid the warning arising with W=n compile option. Fixes: 817b54aa45db ("evm: add evm_inode_setattr to prevent updating an invalid security.evm") # v3.2+ Fixes: c1632a0f1120 ("fs: port ->setattr() to pass mnt_idmap") # v6.3+ Signed-off-by: Roberto Sassu Reviewed-by: Stefan Berger Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index cf24c5255583..b1c2197473a2 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -795,7 +795,9 @@ static int evm_attr_change(struct mnt_idmap *idmap, /** * evm_inode_setattr - prevent updating an invalid EVM extended attribute + * @idmap: idmap of the mount * @dentry: pointer to the affected dentry + * @attr: iattr structure containing the new file attributes * * Permit update of file attributes when files have a valid EVM signature, * except in the case of them having an immutable portable signature. From 447a5688005e5b789633bd080016517a08f9fd8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20G=C3=B6ttsche?= Date: Fri, 2 Jun 2023 15:35:10 +0200 Subject: [PATCH 30/45] selinux: avoid bool as identifier name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Avoid using the identifier `bool` to improve support with future C standards. C23 is about to make `bool` a predefined macro (see N2654). Signed-off-by: Christian Göttsche Signed-off-by: Paul Moore --- security/selinux/ss/conditional.c | 8 ++++---- security/selinux/ss/conditional.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c index e11219fdf9f7..b156c181c3c1 100644 --- a/security/selinux/ss/conditional.c +++ b/security/selinux/ss/conditional.c @@ -38,7 +38,7 @@ static int cond_evaluate_expr(struct policydb *p, struct cond_expr *expr) if (sp == (COND_EXPR_MAXDEPTH - 1)) return -1; sp++; - s[sp] = p->bool_val_to_struct[node->bool - 1]->state; + s[sp] = p->bool_val_to_struct[node->boolean - 1]->state; break; case COND_NOT: if (sp < 0) @@ -366,7 +366,7 @@ static int expr_node_isvalid(struct policydb *p, struct cond_expr_node *expr) return 0; } - if (expr->bool > p->p_bools.nprim) { + if (expr->boolean > p->p_bools.nprim) { pr_err("SELinux: conditional expressions uses unknown bool.\n"); return 0; } @@ -401,7 +401,7 @@ static int cond_read_node(struct policydb *p, struct cond_node *node, void *fp) return rc; expr->expr_type = le32_to_cpu(buf[0]); - expr->bool = le32_to_cpu(buf[1]); + expr->boolean = le32_to_cpu(buf[1]); if (!expr_node_isvalid(p, expr)) return -EINVAL; @@ -518,7 +518,7 @@ static int cond_write_node(struct policydb *p, struct cond_node *node, for (i = 0; i < node->expr.len; i++) { buf[0] = cpu_to_le32(node->expr.nodes[i].expr_type); - buf[1] = cpu_to_le32(node->expr.nodes[i].bool); + buf[1] = cpu_to_le32(node->expr.nodes[i].boolean); rc = put_entry(buf, sizeof(u32), 2, fp); if (rc) return rc; diff --git a/security/selinux/ss/conditional.h b/security/selinux/ss/conditional.h index e47ec6ddeaf6..5a7b51278dc6 100644 --- a/security/selinux/ss/conditional.h +++ b/security/selinux/ss/conditional.h @@ -29,7 +29,7 @@ struct cond_expr_node { #define COND_NEQ 7 /* bool != bool */ #define COND_LAST COND_NEQ u32 expr_type; - u32 bool; + u32 boolean; }; struct cond_expr { From 996e0a97ebd7b11cb785794e2a83c20c1add9d92 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Tue, 6 Jun 2023 09:41:12 +0200 Subject: [PATCH 31/45] evm: Fix build warnings Fix build warnings (function parameters description) for evm_read_protected_xattrs(), evm_set_key() and evm_verifyxattr(). Fixes: 7626676320f3 ("evm: provide a function to set the EVM key from the kernel") # v4.5+ Fixes: 8314b6732ae4 ("ima: Define new template fields xattrnames, xattrlengths and xattrvalues") # v5.14+ Fixes: 2960e6cb5f7c ("evm: additional parameter to pass integrity cache entry 'iint'") # v3.2+ Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_crypto.c | 2 +- security/integrity/evm/evm_main.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 033804f5a5f2..0dae649f3740 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -40,7 +40,7 @@ static const char evm_hmac[] = "hmac(sha1)"; /** * evm_set_key() - set EVM HMAC key from the kernel * @key: pointer to a buffer with the key data - * @size: length of the key data + * @keylen: length of the key data * * This function allows setting the EVM HMAC key from the kernel * without using the "encrypted" key subsystem keys. It can be used diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index b1c2197473a2..c9b6e2a43478 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -318,7 +318,6 @@ int evm_protected_xattr_if_enabled(const char *req_xattr_name) /** * evm_read_protected_xattrs - read EVM protected xattr names, lengths, values * @dentry: dentry of the read xattrs - * @inode: inode of the read xattrs * @buffer: buffer xattr names, lengths or values are copied to * @buffer_size: size of buffer * @type: n: names, l: lengths, v: values @@ -390,6 +389,7 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, * @xattr_name: requested xattr * @xattr_value: requested xattr value * @xattr_value_len: requested xattr value length + * @iint: inode integrity metadata * * Calculate the HMAC for the given dentry and verify it against the stored * security.evm xattr. For performance, use the xattr value and length From 95526d13038c2bbddd567a4d8e39fac42484e182 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Tue, 6 Jun 2023 09:41:13 +0200 Subject: [PATCH 32/45] ima: Fix build warnings Fix build warnings (function parameters description) for ima_collect_modsig(), ima_match_policy() and ima_parse_add_rule(). Fixes: 15588227e086 ("ima: Collect modsig") # v5.4+ Fixes: 2fe5d6def167 ("ima: integrity appraisal extension") # v5.14+ Fixes: 4af4662fa4a9 ("integrity: IMA policy") # v3.2+ Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_modsig.c | 3 +++ security/integrity/ima/ima_policy.c | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c index fb25723c65bc..3e7bee30080f 100644 --- a/security/integrity/ima/ima_modsig.c +++ b/security/integrity/ima/ima_modsig.c @@ -89,6 +89,9 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len, /** * ima_collect_modsig - Calculate the file hash without the appended signature. + * @modsig: parsed module signature + * @buf: data to verify the signature on + * @size: data size * * Since the modsig is part of the file contents, the hash used in its signature * isn't the same one ordinarily calculated by IMA. Therefore PKCS7 code diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 3ca8b7348c2e..c9b3bd8f1bb9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -721,6 +721,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) * @secid: LSM secid of the task to be validated * @func: IMA hook identifier * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC) + * @flags: IMA actions to consider (e.g. IMA_MEASURE | IMA_APPRAISE) * @pcr: set the pcr to extend * @template_desc: the template that should be used for this rule * @func_data: func specific data, may be NULL @@ -1915,7 +1916,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) /** * ima_parse_add_rule - add a rule to ima_policy_rules - * @rule - ima measurement policy rule + * @rule: ima measurement policy rule * * Avoid locking by allowing just one writer at a time in ima_write_policy() * Returns the length of the rule parsed, an error code on failure From 55382134366e641e97cd83264c22c60c7dc10ccd Mon Sep 17 00:00:00 2001 From: "GONG, Ruiqi" Date: Fri, 2 Jun 2023 13:45:27 +0800 Subject: [PATCH 33/45] capability: erase checker warnings about struct __user_cap_data_struct Currently Sparse warns the following when compiling kernel/capability.c: kernel/capability.c:191:35: warning: incorrect type in argument 2 (different address spaces) kernel/capability.c:191:35: expected void const *from kernel/capability.c:191:35: got struct __user_cap_data_struct [noderef] __user * kernel/capability.c:168:14: warning: dereference of noderef expression ...... (multiple noderef warnings on different locations) kernel/capability.c:244:29: warning: incorrect type in argument 1 (different address spaces) kernel/capability.c:244:29: expected void *to kernel/capability.c:244:29: got struct __user_cap_data_struct [noderef] __user ( * )[2] kernel/capability.c:247:42: warning: dereference of noderef expression ...... (multiple noderef warnings on different locations) It seems that defining `struct __user_cap_data_struct` together with `cap_user_data_t` make Sparse believe that the struct is `noderef` as well. Separate their definitions to clarify their respective attributes. Signed-off-by: GONG, Ruiqi Acked-by: Serge Hallyn [PM: wrapped long lines in the description] Signed-off-by: Paul Moore --- include/uapi/linux/capability.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h index 3d61a0ae055d..5bb906098697 100644 --- a/include/uapi/linux/capability.h +++ b/include/uapi/linux/capability.h @@ -41,11 +41,12 @@ typedef struct __user_cap_header_struct { int pid; } __user *cap_user_header_t; -typedef struct __user_cap_data_struct { +struct __user_cap_data_struct { __u32 effective; __u32 permitted; __u32 inheritable; -} __user *cap_user_data_t; +}; +typedef struct __user_cap_data_struct __user *cap_user_data_t; #define VFS_CAP_REVISION_MASK 0xFF000000 From b432b55176443e96b6f3bc39ed140ab00cddd84f Mon Sep 17 00:00:00 2001 From: Paul Moore Date: Thu, 8 Jun 2023 14:58:28 -0400 Subject: [PATCH 34/45] MAINTAINERS: move labeled networking to "supported" My current employer, Microsoft, cares very much about the development and maintenance of the upstream Linux Kernel so we can consider labeled networking to be "supported" according to the definition in MAINTAINERS: S: *Status*, one of the following: Supported: Someone is actually paid to look after this. Maintained: Someone actually looks after it. ... Signed-off-by: Paul Moore --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 7e0b87d5aa2e..8818cd866009 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14667,7 +14667,7 @@ NETWORKING [LABELED] (NetLabel, Labeled IPsec, SECMARK) M: Paul Moore L: netdev@vger.kernel.org L: linux-security-module@vger.kernel.org -S: Maintained +S: Supported W: https://github.com/netlabel F: Documentation/netlabel/ F: include/net/calipso.h From 74ce793bcbde5cef0f82d6ccb3c47cb651295a9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 12 Jun 2023 21:14:25 +0200 Subject: [PATCH 35/45] hostfs: Fix ephemeral inodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hostfs creates a new inode for each opened or created file, which created useless inode allocations and forbade identifying a host file with a kernel inode. Fix this uncommon filesystem behavior by tying kernel inodes to host file's inode and device IDs. Even if the host filesystem inodes may be recycled, this cannot happen while a file referencing it is opened, which is the case with hostfs. It should be noted that hostfs inode IDs may not be unique for the same hostfs superblock because multiple host's (backed) superblocks may be used. Delete inodes when dropping them to force backed host's file descriptors closing. This enables to entirely remove ARCH_EPHEMERAL_INODES, and then makes Landlock fully supported by UML. This is very useful for testing changes. These changes also factor out and simplify some helpers thanks to the new hostfs_inode_update() and the hostfs_iget() revamp: read_name(), hostfs_create(), hostfs_lookup(), hostfs_mknod(), and hostfs_fill_sb_common(). A following commit with new Landlock tests check this new hostfs inode consistency. Cc: Anton Ivanov Cc: Johannes Berg Acked-by: Richard Weinberger Link: https://lore.kernel.org/r/20230612191430.339153-2-mic@digikod.net Signed-off-by: Mickaël Salaün --- arch/Kconfig | 7 -- arch/um/Kconfig | 1 - fs/hostfs/hostfs.h | 1 + fs/hostfs/hostfs_kern.c | 213 +++++++++++++++++++------------------- fs/hostfs/hostfs_user.c | 1 + security/landlock/Kconfig | 2 +- 6 files changed, 109 insertions(+), 116 deletions(-) diff --git a/arch/Kconfig b/arch/Kconfig index 205fd23e0cad..49996ee9e33b 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -1188,13 +1188,6 @@ config COMPAT_32BIT_TIME config ARCH_NO_PREEMPT bool -config ARCH_EPHEMERAL_INODES - def_bool n - help - An arch should select this symbol if it doesn't keep track of inode - instances on its own, but instead relies on something else (e.g. the - host kernel for an UML kernel). - config ARCH_SUPPORTS_RT bool diff --git a/arch/um/Kconfig b/arch/um/Kconfig index 541a9b18e343..4057d5267c6a 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -5,7 +5,6 @@ menu "UML-specific options" config UML bool default y - select ARCH_EPHEMERAL_INODES select ARCH_HAS_FORTIFY_SOURCE select ARCH_HAS_GCOV_PROFILE_ALL select ARCH_HAS_KCOV diff --git a/fs/hostfs/hostfs.h b/fs/hostfs/hostfs.h index 69cb796f6270..0239e3af3945 100644 --- a/fs/hostfs/hostfs.h +++ b/fs/hostfs/hostfs.h @@ -65,6 +65,7 @@ struct hostfs_stat { unsigned long long blocks; unsigned int maj; unsigned int min; + dev_t dev; }; extern int stat_file(const char *path, struct hostfs_stat *p, int fd); diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 28b4f15c19eb..19496f732016 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -26,6 +26,7 @@ struct hostfs_inode_info { fmode_t mode; struct inode vfs_inode; struct mutex open_mutex; + dev_t dev; }; static inline struct hostfs_inode_info *HOSTFS_I(struct inode *inode) @@ -182,14 +183,6 @@ static char *follow_link(char *link) return ERR_PTR(n); } -static struct inode *hostfs_iget(struct super_block *sb) -{ - struct inode *inode = new_inode(sb); - if (!inode) - return ERR_PTR(-ENOMEM); - return inode; -} - static int hostfs_statfs(struct dentry *dentry, struct kstatfs *sf) { /* @@ -228,6 +221,7 @@ static struct inode *hostfs_alloc_inode(struct super_block *sb) return NULL; hi->fd = -1; hi->mode = 0; + hi->dev = 0; inode_init_once(&hi->vfs_inode); mutex_init(&hi->open_mutex); return &hi->vfs_inode; @@ -240,6 +234,7 @@ static void hostfs_evict_inode(struct inode *inode) if (HOSTFS_I(inode)->fd != -1) { close_file(&HOSTFS_I(inode)->fd); HOSTFS_I(inode)->fd = -1; + HOSTFS_I(inode)->dev = 0; } } @@ -265,6 +260,7 @@ static int hostfs_show_options(struct seq_file *seq, struct dentry *root) static const struct super_operations hostfs_sbops = { .alloc_inode = hostfs_alloc_inode, .free_inode = hostfs_free_inode, + .drop_inode = generic_delete_inode, .evict_inode = hostfs_evict_inode, .statfs = hostfs_statfs, .show_options = hostfs_show_options, @@ -512,18 +508,31 @@ static const struct address_space_operations hostfs_aops = { .write_end = hostfs_write_end, }; -static int read_name(struct inode *ino, char *name) +static int hostfs_inode_update(struct inode *ino, const struct hostfs_stat *st) { + set_nlink(ino, st->nlink); + i_uid_write(ino, st->uid); + i_gid_write(ino, st->gid); + ino->i_atime = + (struct timespec64){ st->atime.tv_sec, st->atime.tv_nsec }; + ino->i_mtime = + (struct timespec64){ st->mtime.tv_sec, st->mtime.tv_nsec }; + ino->i_ctime = + (struct timespec64){ st->ctime.tv_sec, st->ctime.tv_nsec }; + ino->i_size = st->size; + ino->i_blocks = st->blocks; + return 0; +} + +static int hostfs_inode_set(struct inode *ino, void *data) +{ + struct hostfs_stat *st = data; dev_t rdev; - struct hostfs_stat st; - int err = stat_file(name, &st, -1); - if (err) - return err; /* Reencode maj and min with the kernel encoding.*/ - rdev = MKDEV(st.maj, st.min); + rdev = MKDEV(st->maj, st->min); - switch (st.mode & S_IFMT) { + switch (st->mode & S_IFMT) { case S_IFLNK: ino->i_op = &hostfs_link_iops; break; @@ -535,7 +544,7 @@ static int read_name(struct inode *ino, char *name) case S_IFBLK: case S_IFIFO: case S_IFSOCK: - init_special_inode(ino, st.mode & S_IFMT, rdev); + init_special_inode(ino, st->mode & S_IFMT, rdev); ino->i_op = &hostfs_iops; break; case S_IFREG: @@ -547,17 +556,42 @@ static int read_name(struct inode *ino, char *name) return -EIO; } - ino->i_ino = st.ino; - ino->i_mode = st.mode; - set_nlink(ino, st.nlink); - i_uid_write(ino, st.uid); - i_gid_write(ino, st.gid); - ino->i_atime = (struct timespec64){ st.atime.tv_sec, st.atime.tv_nsec }; - ino->i_mtime = (struct timespec64){ st.mtime.tv_sec, st.mtime.tv_nsec }; - ino->i_ctime = (struct timespec64){ st.ctime.tv_sec, st.ctime.tv_nsec }; - ino->i_size = st.size; - ino->i_blocks = st.blocks; - return 0; + HOSTFS_I(ino)->dev = st->dev; + ino->i_ino = st->ino; + ino->i_mode = st->mode; + return hostfs_inode_update(ino, st); +} + +static int hostfs_inode_test(struct inode *inode, void *data) +{ + const struct hostfs_stat *st = data; + + return inode->i_ino == st->ino && HOSTFS_I(inode)->dev == st->dev; +} + +static struct inode *hostfs_iget(struct super_block *sb, char *name) +{ + struct inode *inode; + struct hostfs_stat st; + int err = stat_file(name, &st, -1); + + if (err) + return ERR_PTR(err); + + inode = iget5_locked(sb, st.ino, hostfs_inode_test, hostfs_inode_set, + &st); + if (!inode) + return ERR_PTR(-ENOMEM); + + if (inode->i_state & I_NEW) { + unlock_new_inode(inode); + } else { + spin_lock(&inode->i_lock); + hostfs_inode_update(inode, &st); + spin_unlock(&inode->i_lock); + } + + return inode; } static int hostfs_create(struct mnt_idmap *idmap, struct inode *dir, @@ -565,62 +599,48 @@ static int hostfs_create(struct mnt_idmap *idmap, struct inode *dir, { struct inode *inode; char *name; - int error, fd; + int fd; - inode = hostfs_iget(dir->i_sb); - if (IS_ERR(inode)) { - error = PTR_ERR(inode); - goto out; - } - - error = -ENOMEM; name = dentry_name(dentry); if (name == NULL) - goto out_put; + return -ENOMEM; fd = file_create(name, mode & 0777); - if (fd < 0) - error = fd; - else - error = read_name(inode, name); + if (fd < 0) { + __putname(name); + return fd; + } + inode = hostfs_iget(dir->i_sb, name); __putname(name); - if (error) - goto out_put; + if (IS_ERR(inode)) + return PTR_ERR(inode); HOSTFS_I(inode)->fd = fd; HOSTFS_I(inode)->mode = FMODE_READ | FMODE_WRITE; d_instantiate(dentry, inode); return 0; - - out_put: - iput(inode); - out: - return error; } static struct dentry *hostfs_lookup(struct inode *ino, struct dentry *dentry, unsigned int flags) { - struct inode *inode; + struct inode *inode = NULL; char *name; - int err; - inode = hostfs_iget(ino->i_sb); - if (IS_ERR(inode)) - goto out; - - err = -ENOMEM; name = dentry_name(dentry); - if (name) { - err = read_name(inode, name); - __putname(name); + if (name == NULL) + return ERR_PTR(-ENOMEM); + + inode = hostfs_iget(ino->i_sb, name); + __putname(name); + if (IS_ERR(inode)) { + if (PTR_ERR(inode) == -ENOENT) + inode = NULL; + else + return ERR_CAST(inode); } - if (err) { - iput(inode); - inode = (err == -ENOENT) ? NULL : ERR_PTR(err); - } - out: + return d_splice_alias(inode, dentry); } @@ -704,35 +724,23 @@ static int hostfs_mknod(struct mnt_idmap *idmap, struct inode *dir, char *name; int err; - inode = hostfs_iget(dir->i_sb); - if (IS_ERR(inode)) { - err = PTR_ERR(inode); - goto out; - } - - err = -ENOMEM; name = dentry_name(dentry); if (name == NULL) - goto out_put; + return -ENOMEM; err = do_mknod(name, mode, MAJOR(dev), MINOR(dev)); - if (err) - goto out_free; + if (err) { + __putname(name); + return err; + } - err = read_name(inode, name); + inode = hostfs_iget(dir->i_sb, name); __putname(name); - if (err) - goto out_put; + if (IS_ERR(inode)) + return PTR_ERR(inode); d_instantiate(dentry, inode); return 0; - - out_free: - __putname(name); - out_put: - iput(inode); - out: - return err; } static int hostfs_rename2(struct mnt_idmap *idmap, @@ -929,49 +937,40 @@ static int hostfs_fill_sb_common(struct super_block *sb, void *d, int silent) sb->s_maxbytes = MAX_LFS_FILESIZE; err = super_setup_bdi(sb); if (err) - goto out; + return err; /* NULL is printed as '(null)' by printf(): avoid that. */ if (req_root == NULL) req_root = ""; - err = -ENOMEM; sb->s_fs_info = host_root_path = kasprintf(GFP_KERNEL, "%s/%s", root_ino, req_root); if (host_root_path == NULL) - goto out; + return -ENOMEM; - root_inode = new_inode(sb); - if (!root_inode) - goto out; - - err = read_name(root_inode, host_root_path); - if (err) - goto out_put; + root_inode = hostfs_iget(sb, host_root_path); + if (IS_ERR(root_inode)) + return PTR_ERR(root_inode); if (S_ISLNK(root_inode->i_mode)) { - char *name = follow_link(host_root_path); - if (IS_ERR(name)) { - err = PTR_ERR(name); - goto out_put; - } - err = read_name(root_inode, name); + char *name; + + iput(root_inode); + name = follow_link(host_root_path); + if (IS_ERR(name)) + return PTR_ERR(name); + + root_inode = hostfs_iget(sb, name); kfree(name); - if (err) - goto out_put; + if (IS_ERR(root_inode)) + return PTR_ERR(root_inode); } - err = -ENOMEM; sb->s_root = d_make_root(root_inode); if (sb->s_root == NULL) - goto out; + return -ENOMEM; return 0; - -out_put: - iput(root_inode); -out: - return err; } static struct dentry *hostfs_read_sb(struct file_system_type *type, diff --git a/fs/hostfs/hostfs_user.c b/fs/hostfs/hostfs_user.c index 5ecc4706172b..840619e39a1a 100644 --- a/fs/hostfs/hostfs_user.c +++ b/fs/hostfs/hostfs_user.c @@ -36,6 +36,7 @@ static void stat64_to_hostfs(const struct stat64 *buf, struct hostfs_stat *p) p->blocks = buf->st_blocks; p->maj = os_major(buf->st_rdev); p->min = os_minor(buf->st_rdev); + p->dev = buf->st_dev; } int stat_file(const char *path, struct hostfs_stat *p, int fd) diff --git a/security/landlock/Kconfig b/security/landlock/Kconfig index 8e33c4e8ffb8..c1e862a38410 100644 --- a/security/landlock/Kconfig +++ b/security/landlock/Kconfig @@ -2,7 +2,7 @@ config SECURITY_LANDLOCK bool "Landlock support" - depends on SECURITY && !ARCH_EPHEMERAL_INODES + depends on SECURITY select SECURITY_PATH help Landlock is a sandboxing mechanism that enables processes to restrict From 592efeb4a07ee46204c0f87001455c28f4743dea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 12 Jun 2023 21:14:26 +0200 Subject: [PATCH 36/45] selftests/landlock: Don't create useless file layouts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add and use a layout0 test fixture to not populate the tmpfs filesystem if it is not required for tests: unknown_access_rights, proc_nsfs, unpriv and max_layers. This doesn't change these tests but it speeds up their setup and makes them less prone to error. This prepare the ground for a next commit. Link: https://lore.kernel.org/r/20230612191430.339153-3-mic@digikod.net Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/fs_test.c | 26 +++++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index b6c4be3faf7a..0438651f61d2 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -231,6 +231,20 @@ static void cleanup_layout(struct __test_metadata *const _metadata) EXPECT_EQ(0, remove_path(TMP_DIR)); } +/* clang-format off */ +FIXTURE(layout0) {}; +/* clang-format on */ + +FIXTURE_SETUP(layout0) +{ + prepare_layout(_metadata); +} + +FIXTURE_TEARDOWN(layout0) +{ + cleanup_layout(_metadata); +} + static void create_layout1(struct __test_metadata *const _metadata) { create_file(_metadata, file1_s1d1); @@ -510,7 +524,7 @@ TEST_F_FORK(layout1, file_and_dir_access_rights) ASSERT_EQ(0, close(ruleset_fd)); } -TEST_F_FORK(layout1, unknown_access_rights) +TEST_F_FORK(layout0, unknown_access_rights) { __u64 access_mask; @@ -608,7 +622,7 @@ static void enforce_ruleset(struct __test_metadata *const _metadata, } } -TEST_F_FORK(layout1, proc_nsfs) +TEST_F_FORK(layout0, proc_nsfs) { const struct rule rules[] = { { @@ -657,11 +671,11 @@ TEST_F_FORK(layout1, proc_nsfs) ASSERT_EQ(0, close(path_beneath.parent_fd)); } -TEST_F_FORK(layout1, unpriv) +TEST_F_FORK(layout0, unpriv) { const struct rule rules[] = { { - .path = dir_s1d2, + .path = TMP_DIR, .access = ACCESS_RO, }, {}, @@ -1301,12 +1315,12 @@ TEST_F_FORK(layout1, inherit_superset) ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY)); } -TEST_F_FORK(layout1, max_layers) +TEST_F_FORK(layout0, max_layers) { int i, err; const struct rule rules[] = { { - .path = dir_s1d2, + .path = TMP_DIR, .access = ACCESS_RO, }, {}, From 3de64b656b3c827d7d194558b0ae3a09400b116d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 12 Jun 2023 21:14:27 +0200 Subject: [PATCH 37/45] selftests/landlock: Add supports_filesystem() helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace supports_overlayfs() with supports_filesystem() to be able to check several filesystems. This will be useful in a following commit. Only check for overlay filesystem once in the setup step, and then rely on self->skip_test. Cc: Guenter Roeck Cc: Jeff Xu Link: https://lore.kernel.org/r/20230612191430.339153-4-mic@digikod.net Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/fs_test.c | 36 ++++++++++++++-------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 0438651f61d2..c1e655fc06bb 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -107,8 +107,10 @@ static bool fgrep(FILE *const inf, const char *const str) return false; } -static bool supports_overlayfs(void) +static bool supports_filesystem(const char *const filesystem) { + char str[32]; + int len; bool res; FILE *const inf = fopen("/proc/filesystems", "r"); @@ -119,7 +121,12 @@ static bool supports_overlayfs(void) if (!inf) return true; - res = fgrep(inf, "nodev\toverlay\n"); + len = snprintf(str, sizeof(str), "nodev\t%s\n", filesystem); + if (len >= sizeof(str)) + /* Ignores too-long filesystem names. */ + return true; + + res = fgrep(inf, str); fclose(inf); return res; } @@ -4044,14 +4051,17 @@ static const char (*merge_sub_files[])[] = { * └── work */ -/* clang-format off */ -FIXTURE(layout2_overlay) {}; -/* clang-format on */ +FIXTURE(layout2_overlay) +{ + bool skip_test; +}; FIXTURE_SETUP(layout2_overlay) { - if (!supports_overlayfs()) - SKIP(return, "overlayfs is not supported"); + if (!supports_filesystem("overlay")) { + self->skip_test = true; + SKIP(return, "overlayfs is not supported (setup)"); + } prepare_layout(_metadata); @@ -4089,8 +4099,8 @@ FIXTURE_SETUP(layout2_overlay) FIXTURE_TEARDOWN(layout2_overlay) { - if (!supports_overlayfs()) - SKIP(return, "overlayfs is not supported"); + if (self->skip_test) + SKIP(return, "overlayfs is not supported (teardown)"); EXPECT_EQ(0, remove_path(lower_do1_fl3)); EXPECT_EQ(0, remove_path(lower_dl1_fl2)); @@ -4123,8 +4133,8 @@ FIXTURE_TEARDOWN(layout2_overlay) TEST_F_FORK(layout2_overlay, no_restriction) { - if (!supports_overlayfs()) - SKIP(return, "overlayfs is not supported"); + if (self->skip_test) + SKIP(return, "overlayfs is not supported (test)"); ASSERT_EQ(0, test_open(lower_fl1, O_RDONLY)); ASSERT_EQ(0, test_open(lower_dl1, O_RDONLY)); @@ -4289,8 +4299,8 @@ TEST_F_FORK(layout2_overlay, same_content_different_file) size_t i; const char *path_entry; - if (!supports_overlayfs()) - SKIP(return, "overlayfs is not supported"); + if (self->skip_test) + SKIP(return, "overlayfs is not supported (test)"); /* Sets rules on base directories (i.e. outside overlay scope). */ ruleset_fd = create_ruleset(_metadata, ACCESS_RW, layer1_base); From 55ab3fbe8333148e9c21477ffdc4edbe1f568f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 12 Jun 2023 21:14:28 +0200 Subject: [PATCH 38/45] selftests/landlock: Make mounts configurable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a new struct mnt_opt to define a mount point with the mount_opt() helper. This doesn't change tests but prepare for the next commit. Link: https://lore.kernel.org/r/20230612191430.339153-5-mic@digikod.net Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/fs_test.c | 45 +++++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index c1e655fc06bb..737047a81547 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -213,7 +213,26 @@ out: return err; } -static void prepare_layout(struct __test_metadata *const _metadata) +struct mnt_opt { + const char *const source; + const char *const type; + const unsigned long flags; + const char *const data; +}; + +const struct mnt_opt mnt_tmp = { + .type = "tmpfs", + .data = "size=4m,mode=700", +}; + +static int mount_opt(const struct mnt_opt *const mnt, const char *const target) +{ + return mount(mnt->source ?: mnt->type, target, mnt->type, mnt->flags, + mnt->data); +} + +static void prepare_layout_opt(struct __test_metadata *const _metadata, + const struct mnt_opt *const mnt) { disable_caps(_metadata); umask(0077); @@ -225,11 +244,27 @@ static void prepare_layout(struct __test_metadata *const _metadata) */ set_cap(_metadata, CAP_SYS_ADMIN); ASSERT_EQ(0, unshare(CLONE_NEWNS)); - ASSERT_EQ(0, mount("tmp", TMP_DIR, "tmpfs", 0, "size=4m,mode=700")); + ASSERT_EQ(0, mount_opt(mnt, TMP_DIR)) + { + TH_LOG("Failed to mount the %s filesystem: %s", mnt->type, + strerror(errno)); + /* + * FIXTURE_TEARDOWN() is not called when FIXTURE_SETUP() + * failed, so we need to explicitly do a minimal cleanup to + * avoid cascading errors with other tests that don't depend on + * the same filesystem. + */ + remove_path(TMP_DIR); + } ASSERT_EQ(0, mount(NULL, TMP_DIR, NULL, MS_PRIVATE | MS_REC, NULL)); clear_cap(_metadata, CAP_SYS_ADMIN); } +static void prepare_layout(struct __test_metadata *const _metadata) +{ + prepare_layout_opt(_metadata, &mnt_tmp); +} + static void cleanup_layout(struct __test_metadata *const _metadata) { set_cap(_metadata, CAP_SYS_ADMIN); @@ -269,7 +304,7 @@ static void create_layout1(struct __test_metadata *const _metadata) create_file(_metadata, file1_s3d1); create_directory(_metadata, dir_s3d2); set_cap(_metadata, CAP_SYS_ADMIN); - ASSERT_EQ(0, mount("tmp", dir_s3d2, "tmpfs", 0, "size=4m,mode=700")); + ASSERT_EQ(0, mount_opt(&mnt_tmp, dir_s3d2)); clear_cap(_metadata, CAP_SYS_ADMIN); ASSERT_EQ(0, mkdir(dir_s3d3, 0700)); @@ -4068,7 +4103,7 @@ FIXTURE_SETUP(layout2_overlay) create_directory(_metadata, LOWER_BASE); set_cap(_metadata, CAP_SYS_ADMIN); /* Creates tmpfs mount points to get deterministic overlayfs. */ - ASSERT_EQ(0, mount("tmp", LOWER_BASE, "tmpfs", 0, "size=4m,mode=700")); + ASSERT_EQ(0, mount_opt(&mnt_tmp, LOWER_BASE)); clear_cap(_metadata, CAP_SYS_ADMIN); create_file(_metadata, lower_fl1); create_file(_metadata, lower_dl1_fl2); @@ -4078,7 +4113,7 @@ FIXTURE_SETUP(layout2_overlay) create_directory(_metadata, UPPER_BASE); set_cap(_metadata, CAP_SYS_ADMIN); - ASSERT_EQ(0, mount("tmp", UPPER_BASE, "tmpfs", 0, "size=4m,mode=700")); + ASSERT_EQ(0, mount_opt(&mnt_tmp, UPPER_BASE)); clear_cap(_metadata, CAP_SYS_ADMIN); create_file(_metadata, upper_fu1); create_file(_metadata, upper_du1_fu2); From 04f9070e99a4c5d60f05436f63e470c19b579e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 12 Jun 2023 21:14:29 +0200 Subject: [PATCH 39/45] selftests/landlock: Add tests for pseudo filesystems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add generic and read-only tests for 6 pseudo filesystems to make sure they have a consistent inode management, which is required for Landlock's file hierarchy identification: - tmpfs - ramfs - cgroup2 - proc - sysfs Update related kernel configuration to support these new filesystems, remove useless CONFIG_SECURITY_PATH, and sort all entries. If these filesystems are not supported by the kernel running tests, the related tests are skipped. Expanding variants, this adds 25 new tests for layout3_fs: - tag_inode_dir_parent - tag_inode_dir_mnt - tag_inode_dir_child - tag_inode_dir_file - release_inodes Test coverage for security/landlock with kernel debug code: - 94.7% of 835 lines according to gcc/gcov-12 - 93.0% of 852 lines according to gcc/gcov-13 Test coverage for security/landlock without kernel debug code: - 95.5% of 624 lines according to gcc/gcov-12 - 93.1% of 641 lines according to gcc/gcov-13 Link: https://lore.kernel.org/r/20230612191430.339153-6-mic@digikod.net Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/config | 9 +- tools/testing/selftests/landlock/fs_test.c | 254 ++++++++++++++++++++- 2 files changed, 259 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/landlock/config b/tools/testing/selftests/landlock/config index 0f0a65287bac..3dc9e438eab1 100644 --- a/tools/testing/selftests/landlock/config +++ b/tools/testing/selftests/landlock/config @@ -1,7 +1,10 @@ +CONFIG_CGROUPS=y +CONFIG_CGROUP_SCHED=y CONFIG_OVERLAY_FS=y -CONFIG_SECURITY_LANDLOCK=y -CONFIG_SECURITY_PATH=y +CONFIG_PROC_FS=y CONFIG_SECURITY=y +CONFIG_SECURITY_LANDLOCK=y CONFIG_SHMEM=y -CONFIG_TMPFS_XATTR=y +CONFIG_SYSFS=y CONFIG_TMPFS=y +CONFIG_TMPFS_XATTR=y diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 737047a81547..2911b5241583 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -121,6 +121,10 @@ static bool supports_filesystem(const char *const filesystem) if (!inf) return true; + /* filesystem can be null for bind mounts. */ + if (!filesystem) + return true; + len = snprintf(str, sizeof(str), "nodev\t%s\n", filesystem); if (len >= sizeof(str)) /* Ignores too-long filesystem names. */ @@ -243,7 +247,7 @@ static void prepare_layout_opt(struct __test_metadata *const _metadata, * for tests relying on pivot_root(2) and move_mount(2). */ set_cap(_metadata, CAP_SYS_ADMIN); - ASSERT_EQ(0, unshare(CLONE_NEWNS)); + ASSERT_EQ(0, unshare(CLONE_NEWNS | CLONE_NEWCGROUP)); ASSERT_EQ(0, mount_opt(mnt, TMP_DIR)) { TH_LOG("Failed to mount the %s filesystem: %s", mnt->type, @@ -318,11 +322,13 @@ static void remove_layout1(struct __test_metadata *const _metadata) EXPECT_EQ(0, remove_path(file1_s1d3)); EXPECT_EQ(0, remove_path(file1_s1d2)); EXPECT_EQ(0, remove_path(file1_s1d1)); + EXPECT_EQ(0, remove_path(dir_s1d3)); EXPECT_EQ(0, remove_path(file2_s2d3)); EXPECT_EQ(0, remove_path(file1_s2d3)); EXPECT_EQ(0, remove_path(file1_s2d2)); EXPECT_EQ(0, remove_path(file1_s2d1)); + EXPECT_EQ(0, remove_path(dir_s2d2)); EXPECT_EQ(0, remove_path(file1_s3d1)); EXPECT_EQ(0, remove_path(dir_s3d3)); @@ -4482,4 +4488,250 @@ TEST_F_FORK(layout2_overlay, same_content_different_file) } } +FIXTURE(layout3_fs) +{ + bool has_created_dir; + bool has_created_file; + char *dir_path; + bool skip_test; +}; + +FIXTURE_VARIANT(layout3_fs) +{ + const struct mnt_opt mnt; + const char *const file_path; +}; + +/* clang-format off */ +FIXTURE_VARIANT_ADD(layout3_fs, tmpfs) { + /* clang-format on */ + .mnt = mnt_tmp, + .file_path = file1_s1d1, +}; + +FIXTURE_VARIANT_ADD(layout3_fs, ramfs) { + .mnt = { + .type = "ramfs", + .data = "mode=700", + }, + .file_path = TMP_DIR "/dir/file", +}; + +FIXTURE_VARIANT_ADD(layout3_fs, cgroup2) { + .mnt = { + .type = "cgroup2", + }, + .file_path = TMP_DIR "/test/cgroup.procs", +}; + +FIXTURE_VARIANT_ADD(layout3_fs, proc) { + .mnt = { + .type = "proc", + }, + .file_path = TMP_DIR "/self/status", +}; + +FIXTURE_VARIANT_ADD(layout3_fs, sysfs) { + .mnt = { + .type = "sysfs", + }, + .file_path = TMP_DIR "/kernel/notes", +}; + +FIXTURE_SETUP(layout3_fs) +{ + struct stat statbuf; + const char *slash; + size_t dir_len; + + if (!supports_filesystem(variant->mnt.type)) { + self->skip_test = true; + SKIP(return, "this filesystem is not supported (setup)"); + } + + slash = strrchr(variant->file_path, '/'); + ASSERT_NE(slash, NULL); + dir_len = (size_t)slash - (size_t)variant->file_path; + ASSERT_LT(0, dir_len); + self->dir_path = malloc(dir_len + 1); + self->dir_path[dir_len] = '\0'; + strncpy(self->dir_path, variant->file_path, dir_len); + + prepare_layout_opt(_metadata, &variant->mnt); + + /* Creates directory when required. */ + if (stat(self->dir_path, &statbuf)) { + set_cap(_metadata, CAP_DAC_OVERRIDE); + EXPECT_EQ(0, mkdir(self->dir_path, 0700)) + { + TH_LOG("Failed to create directory \"%s\": %s", + self->dir_path, strerror(errno)); + free(self->dir_path); + self->dir_path = NULL; + } + self->has_created_dir = true; + clear_cap(_metadata, CAP_DAC_OVERRIDE); + } + + /* Creates file when required. */ + if (stat(variant->file_path, &statbuf)) { + int fd; + + set_cap(_metadata, CAP_DAC_OVERRIDE); + fd = creat(variant->file_path, 0600); + EXPECT_LE(0, fd) + { + TH_LOG("Failed to create file \"%s\": %s", + variant->file_path, strerror(errno)); + } + EXPECT_EQ(0, close(fd)); + self->has_created_file = true; + clear_cap(_metadata, CAP_DAC_OVERRIDE); + } +} + +FIXTURE_TEARDOWN(layout3_fs) +{ + if (self->skip_test) + SKIP(return, "this filesystem is not supported (teardown)"); + + if (self->has_created_file) { + set_cap(_metadata, CAP_DAC_OVERRIDE); + /* + * Don't check for error because the file might already + * have been removed (cf. release_inode test). + */ + unlink(variant->file_path); + clear_cap(_metadata, CAP_DAC_OVERRIDE); + } + + if (self->has_created_dir) { + set_cap(_metadata, CAP_DAC_OVERRIDE); + /* + * Don't check for error because the directory might already + * have been removed (cf. release_inode test). + */ + rmdir(self->dir_path); + clear_cap(_metadata, CAP_DAC_OVERRIDE); + } + free(self->dir_path); + self->dir_path = NULL; + + cleanup_layout(_metadata); +} + +static void layer3_fs_tag_inode(struct __test_metadata *const _metadata, + FIXTURE_DATA(layout3_fs) * self, + const FIXTURE_VARIANT(layout3_fs) * variant, + const char *const rule_path) +{ + const struct rule layer1_allow_read_file[] = { + { + .path = rule_path, + .access = LANDLOCK_ACCESS_FS_READ_FILE, + }, + {}, + }; + const struct landlock_ruleset_attr layer2_deny_everything_attr = { + .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE, + }; + const char *const dev_null_path = "/dev/null"; + int ruleset_fd; + + if (self->skip_test) + SKIP(return, "this filesystem is not supported (test)"); + + /* Checks without Landlock. */ + EXPECT_EQ(0, test_open(dev_null_path, O_RDONLY | O_CLOEXEC)); + EXPECT_EQ(0, test_open(variant->file_path, O_RDONLY | O_CLOEXEC)); + + ruleset_fd = create_ruleset(_metadata, LANDLOCK_ACCESS_FS_READ_FILE, + layer1_allow_read_file); + EXPECT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + EXPECT_EQ(0, close(ruleset_fd)); + + EXPECT_EQ(EACCES, test_open(dev_null_path, O_RDONLY | O_CLOEXEC)); + EXPECT_EQ(0, test_open(variant->file_path, O_RDONLY | O_CLOEXEC)); + + /* Forbids directory reading. */ + ruleset_fd = + landlock_create_ruleset(&layer2_deny_everything_attr, + sizeof(layer2_deny_everything_attr), 0); + EXPECT_LE(0, ruleset_fd); + enforce_ruleset(_metadata, ruleset_fd); + EXPECT_EQ(0, close(ruleset_fd)); + + /* Checks with Landlock and forbidden access. */ + EXPECT_EQ(EACCES, test_open(dev_null_path, O_RDONLY | O_CLOEXEC)); + EXPECT_EQ(EACCES, test_open(variant->file_path, O_RDONLY | O_CLOEXEC)); +} + +/* Matrix of tests to check file hierarchy evaluation. */ + +TEST_F_FORK(layout3_fs, tag_inode_dir_parent) +{ + /* The current directory must not be the root for this test. */ + layer3_fs_tag_inode(_metadata, self, variant, "."); +} + +TEST_F_FORK(layout3_fs, tag_inode_dir_mnt) +{ + layer3_fs_tag_inode(_metadata, self, variant, TMP_DIR); +} + +TEST_F_FORK(layout3_fs, tag_inode_dir_child) +{ + layer3_fs_tag_inode(_metadata, self, variant, self->dir_path); +} + +TEST_F_FORK(layout3_fs, tag_inode_file) +{ + layer3_fs_tag_inode(_metadata, self, variant, variant->file_path); +} + +/* Light version of layout1.release_inodes */ +TEST_F_FORK(layout3_fs, release_inodes) +{ + const struct rule layer1[] = { + { + .path = TMP_DIR, + .access = LANDLOCK_ACCESS_FS_READ_DIR, + }, + {}, + }; + int ruleset_fd; + + if (self->skip_test) + SKIP(return, "this filesystem is not supported (test)"); + + /* Clean up for the teardown to not fail. */ + if (self->has_created_file) + EXPECT_EQ(0, remove_path(variant->file_path)); + + if (self->has_created_dir) + /* Don't check for error because of cgroup specificities. */ + remove_path(self->dir_path); + + ruleset_fd = + create_ruleset(_metadata, LANDLOCK_ACCESS_FS_READ_DIR, layer1); + ASSERT_LE(0, ruleset_fd); + + /* Unmount the filesystem while it is being used by a ruleset. */ + set_cap(_metadata, CAP_SYS_ADMIN); + ASSERT_EQ(0, umount(TMP_DIR)); + clear_cap(_metadata, CAP_SYS_ADMIN); + + /* Replaces with a new mount point to simplify FIXTURE_TEARDOWN. */ + set_cap(_metadata, CAP_SYS_ADMIN); + ASSERT_EQ(0, mount_opt(&mnt_tmp, TMP_DIR)); + clear_cap(_metadata, CAP_SYS_ADMIN); + + enforce_ruleset(_metadata, ruleset_fd); + ASSERT_EQ(0, close(ruleset_fd)); + + /* Checks that access to the new mount point is denied. */ + ASSERT_EQ(EACCES, test_open(TMP_DIR, O_RDONLY)); +} + TEST_HARNESS_MAIN From 35ca4239929737bdc021ee923f97ebe7aff8fcc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Mon, 12 Jun 2023 21:14:30 +0200 Subject: [PATCH 40/45] selftests/landlock: Add hostfs tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests for the hostfs filesystems to make sure it has a consistent inode management, which is required for Landlock's file hierarchy identification. This adds 5 new tests for layout3_fs with the hostfs variant. Add hostfs to the new (architecture-specific) config.um file. The hostfs filesystem, only available for an User-Mode Linux kernel, is special because we cannot explicitly mount it. The layout3_fs.hostfs variant tests are skipped if the current test directory is not backed by this filesystem. The layout3_fs.hostfs.tag_inode_dir_child and layout3_fs.hostfs.tag_inode_file tests pass thanks to a previous commit fixing hostfs inode management. Without this fix, the deny-by-default policy would apply and all access requests would be denied. Link: https://lore.kernel.org/r/20230612191430.339153-7-mic@digikod.net Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/config.um | 1 + tools/testing/selftests/landlock/fs_test.c | 28 +++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/landlock/config.um diff --git a/tools/testing/selftests/landlock/config.um b/tools/testing/selftests/landlock/config.um new file mode 100644 index 000000000000..40937c0395d6 --- /dev/null +++ b/tools/testing/selftests/landlock/config.um @@ -0,0 +1 @@ +CONFIG_HOSTFS=y diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 2911b5241583..83d565569512 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -10,6 +10,7 @@ #define _GNU_SOURCE #include #include +#include #include #include #include @@ -19,6 +20,7 @@ #include #include #include +#include #include #include "common.h" @@ -135,6 +137,19 @@ static bool supports_filesystem(const char *const filesystem) return res; } +static bool cwd_matches_fs(unsigned int fs_magic) +{ + struct statfs statfs_buf; + + if (!fs_magic) + return true; + + if (statfs(".", &statfs_buf)) + return true; + + return statfs_buf.f_type == fs_magic; +} + static void mkdir_parents(struct __test_metadata *const _metadata, const char *const path) { @@ -4500,6 +4515,7 @@ FIXTURE_VARIANT(layout3_fs) { const struct mnt_opt mnt; const char *const file_path; + unsigned int cwd_fs_magic; }; /* clang-format off */ @@ -4538,13 +4554,23 @@ FIXTURE_VARIANT_ADD(layout3_fs, sysfs) { .file_path = TMP_DIR "/kernel/notes", }; +FIXTURE_VARIANT_ADD(layout3_fs, hostfs) { + .mnt = { + .source = TMP_DIR, + .flags = MS_BIND, + }, + .file_path = TMP_DIR "/dir/file", + .cwd_fs_magic = HOSTFS_SUPER_MAGIC, +}; + FIXTURE_SETUP(layout3_fs) { struct stat statbuf; const char *slash; size_t dir_len; - if (!supports_filesystem(variant->mnt.type)) { + if (!supports_filesystem(variant->mnt.type) || + !cwd_matches_fs(variant->cwd_fs_magic)) { self->skip_test = true; SKIP(return, "this filesystem is not supported (setup)"); } From d97038d5ec2062733c1e016caf9baaf68cf64ea1 Mon Sep 17 00:00:00 2001 From: Jiasheng Jiang Date: Wed, 14 Jun 2023 17:37:33 +0800 Subject: [PATCH 41/45] pstore/ram: Add check for kstrdup Add check for the return value of kstrdup() and return the error if it fails in order to avoid NULL pointer dereference. Fixes: e163fdb3f7f8 ("pstore/ram: Regularize prz label allocation lifetime") Signed-off-by: Jiasheng Jiang Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230614093733.36048-1-jiasheng@iscas.ac.cn --- fs/pstore/ram_core.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index 966191d3a5ba..85aaf0fc6d7d 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -599,6 +599,8 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, raw_spin_lock_init(&prz->buffer_lock); prz->flags = flags; prz->label = kstrdup(label, GFP_KERNEL); + if (!prz->label) + goto err; ret = persistent_ram_buffer_map(start, size, prz, memtype); if (ret) From 970ebb8a26a120340dcbb4e6c2fb4ecfbad0d190 Mon Sep 17 00:00:00 2001 From: Alexander Mikhalitsyn Date: Wed, 3 May 2023 08:43:44 +0200 Subject: [PATCH 42/45] SafeSetID: fix UID printed instead of GID pr_warn message clearly says that GID should be printed, but we have UID there. Let's fix that. Found accidentally during the work on isolated user namespaces. Signed-off-by: Alexander Mikhalitsyn [PM: fix spelling errors in description, subject tweak] Signed-off-by: Paul Moore --- security/safesetid/lsm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c index e806739f7868..5be5894aa0ea 100644 --- a/security/safesetid/lsm.c +++ b/security/safesetid/lsm.c @@ -131,7 +131,7 @@ static int safesetid_security_capable(const struct cred *cred, * set*gid() (e.g. setting up userns gid mappings). */ pr_warn("Operation requires CAP_SETGID, which is not available to GID %u for operations besides approved set*gid transitions\n", - __kuid_val(cred->uid)); + __kgid_val(cred->gid)); return -EPERM; default: /* Error, the only capabilities were checking for is CAP_SETUID/GID */ From 4be22f16a4a1a1667e79b52b56cca2c64b3747e2 Mon Sep 17 00:00:00 2001 From: Gaosheng Cui Date: Wed, 21 Jun 2023 15:44:18 +0800 Subject: [PATCH 43/45] device_cgroup: Fix kernel-doc warnings in device_cgroup Fix kernel-doc warnings in device_cgroup: security/device_cgroup.c:835: warning: Excess function parameter 'dev_cgroup' description in 'devcgroup_legacy_check_permission'. Signed-off-by: Gaosheng Cui Signed-off-by: Paul Moore --- security/device_cgroup.c | 1 - 1 file changed, 1 deletion(-) diff --git a/security/device_cgroup.c b/security/device_cgroup.c index 41fca6487ca3..dc4df7475081 100644 --- a/security/device_cgroup.c +++ b/security/device_cgroup.c @@ -822,7 +822,6 @@ struct cgroup_subsys devices_cgrp_subsys = { /** * devcgroup_legacy_check_permission - checks if an inode operation is permitted - * @dev_cgroup: the dev cgroup to be tested against * @type: device type * @major: device major number * @minor: device minor number From 0b3d412798a4763aaf39213a279e453e1fddc81d Mon Sep 17 00:00:00 2001 From: Baruch Siach Date: Fri, 23 Jun 2023 08:50:11 +0300 Subject: [PATCH 44/45] elf: correct note name comment NT_PRFPREG note is named "CORE". Correct the comment accordingly. Fixes: 00e19ceec80b ("ELF: Add ELF program property parsing support") Signed-off-by: Baruch Siach Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/455b22b986de4d3bc6d9bfd522378e442943de5f.1687499411.git.baruch@tkos.co.il --- include/uapi/linux/elf.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h index ac3da855fb19..4d1c8d46e7f0 100644 --- a/include/uapi/linux/elf.h +++ b/include/uapi/linux/elf.h @@ -372,7 +372,8 @@ typedef struct elf64_shdr { * Notes used in ET_CORE. Architectures export some of the arch register sets * using the corresponding note types via the PTRACE_GETREGSET and * PTRACE_SETREGSET requests. - * The note name for all these is "LINUX". + * The note name for these types is "LINUX", except NT_PRFPREG that is named + * "CORE". */ #define NT_PRSTATUS 1 #define NT_PRFPREG 2 From aa88054b70905069d1cf706aa5e9a3418d1d341d Mon Sep 17 00:00:00 2001 From: Baruch Siach Date: Fri, 23 Jun 2023 08:56:44 +0300 Subject: [PATCH 45/45] binfmt_elf: fix comment typo s/reset/regset/ Signed-off-by: Baruch Siach Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/0b2967c4a4141875c493e835d5a6f8f2d19ae2d6.1687499804.git.baruch@tkos.co.il --- fs/binfmt_elf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 44b4c42ab8e8..983ce34115d5 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1773,7 +1773,7 @@ static int fill_thread_core_info(struct elf_thread_core_info *t, /* * NT_PRSTATUS is the one special case, because the regset data * goes into the pr_reg field inside the note contents, rather - * than being the whole note contents. We fill the reset in here. + * than being the whole note contents. We fill the regset in here. * We assume that regset 0 is NT_PRSTATUS. */ fill_prstatus(&t->prstatus.common, t->task, signr);