From 460d95a1d69d5c0352379a3651c9cb6ec09e4ddb Mon Sep 17 00:00:00 2001 From: Vishal Goel Date: Thu, 7 Mar 2019 16:55:24 +0530 Subject: smack: removal of global rule list In this patch, global rule list has been removed. Now all smack rules will be read using "smack_known_list". This list contains all the smack labels and internally each smack label structure maintains the list of smack rules corresponding to that smack label. So there is no need to maintain extra list. 1) Small Memory Optimization For eg. if there are 20000 rules, then it will save 625KB(20000*32), which is critical for small embedded systems. 2) Reducing the time taken in writing rules on load/load2 interface 3) Since global rule list is just used to read the rules, so there will be no performance impact on system Signed-off-by: Vishal Goel Signed-off-by: Amit Sahrawat Signed-off-by: Casey Schaufler --- security/smack/smackfs.c | 53 ++++++++++++++---------------------------------- 1 file changed, 15 insertions(+), 38 deletions(-) diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index faf2ea3968b3..8406738b45f2 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -67,7 +67,6 @@ enum smk_inos { /* * List locks */ -static DEFINE_MUTEX(smack_master_list_lock); static DEFINE_MUTEX(smack_cipso_lock); static DEFINE_MUTEX(smack_ambient_lock); static DEFINE_MUTEX(smk_net4addr_lock); @@ -134,15 +133,7 @@ LIST_HEAD(smk_net6addr_list); /* * Rule lists are maintained for each label. - * This master list is just for reading /smack/load and /smack/load2. */ -struct smack_master_list { - struct list_head list; - struct smack_rule *smk_rule; -}; - -static LIST_HEAD(smack_rule_list); - struct smack_parsed_rule { struct smack_known *smk_subject; struct smack_known *smk_object; @@ -211,7 +202,6 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap) * @srp: the rule to add or replace * @rule_list: the list of rules * @rule_lock: the rule list lock - * @global: if non-zero, indicates a global rule * * Looks through the current subject/object/access list for * the subject/object pair and replaces the access that was @@ -223,10 +213,9 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap) */ static int smk_set_access(struct smack_parsed_rule *srp, struct list_head *rule_list, - struct mutex *rule_lock, int global) + struct mutex *rule_lock) { struct smack_rule *sp; - struct smack_master_list *smlp; int found = 0; int rc = 0; @@ -258,22 +247,6 @@ static int smk_set_access(struct smack_parsed_rule *srp, sp->smk_access = srp->smk_access1 & ~srp->smk_access2; list_add_rcu(&sp->list, rule_list); - /* - * If this is a global as opposed to self and a new rule - * it needs to get added for reporting. - */ - if (global) { - mutex_unlock(rule_lock); - smlp = kzalloc(sizeof(*smlp), GFP_KERNEL); - if (smlp != NULL) { - smlp->smk_rule = sp; - mutex_lock(&smack_master_list_lock); - list_add_rcu(&smlp->list, &smack_rule_list); - mutex_unlock(&smack_master_list_lock); - } else - rc = -ENOMEM; - return rc; - } } out: @@ -540,9 +513,9 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf, if (rule_list == NULL) rc = smk_set_access(&rule, &rule.smk_subject->smk_rules, - &rule.smk_subject->smk_rules_lock, 1); + &rule.smk_subject->smk_rules_lock); else - rc = smk_set_access(&rule, rule_list, rule_lock, 0); + rc = smk_set_access(&rule, rule_list, rule_lock); if (rc) goto out; @@ -636,21 +609,23 @@ static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max) static void *load2_seq_start(struct seq_file *s, loff_t *pos) { - return smk_seq_start(s, pos, &smack_rule_list); + return smk_seq_start(s, pos, &smack_known_list); } static void *load2_seq_next(struct seq_file *s, void *v, loff_t *pos) { - return smk_seq_next(s, v, pos, &smack_rule_list); + return smk_seq_next(s, v, pos, &smack_known_list); } static int load_seq_show(struct seq_file *s, void *v) { struct list_head *list = v; - struct smack_master_list *smlp = - list_entry_rcu(list, struct smack_master_list, list); + struct smack_rule *srp; + struct smack_known *skp = + list_entry_rcu(list, struct smack_known, list); - smk_rule_show(s, smlp->smk_rule, SMK_LABELLEN); + list_for_each_entry_rcu(srp, &skp->smk_rules, list) + smk_rule_show(s, srp, SMK_LABELLEN); return 0; } @@ -2352,10 +2327,12 @@ static const struct file_operations smk_access_ops = { static int load2_seq_show(struct seq_file *s, void *v) { struct list_head *list = v; - struct smack_master_list *smlp = - list_entry_rcu(list, struct smack_master_list, list); + struct smack_rule *srp; + struct smack_known *skp = + list_entry_rcu(list, struct smack_known, list); - smk_rule_show(s, smlp->smk_rule, SMK_LONGLABEL); + list_for_each_entry_rcu(srp, &skp->smk_rules, list) + smk_rule_show(s, srp, SMK_LONGLABEL); return 0; } -- cgit v1.2.3-58-ga151 From 4e328b08882a68649df2c2b76fd208b0d0b85503 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Tue, 2 Apr 2019 11:37:12 -0700 Subject: Smack: Create smack_rule cache to optimize memory usage This patch allows for small memory optimization by creating the kmem cache for "struct smack_rule" instead of using kzalloc. For adding new smack rule, kzalloc is used to allocate the memory for "struct smack_rule". kzalloc will always allocate 32 or 64 bytes for 1 structure depending upon the kzalloc cache sizes available in system. Although the size of structure is 20 bytes only, resulting in memory wastage per object in the default pool. For e.g., if there are 20000 rules, then it will save 240KB(20000*12) which is crucial for small memory targets. Signed-off-by: Vishal Goel Signed-off-by: Amit Sahrawat Signed-off-by: Casey Schaufler --- security/smack/smack.h | 1 + security/smack/smack_lsm.c | 11 +++++++++-- security/smack/smackfs.c | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/security/smack/smack.h b/security/smack/smack.h index cf52af77d15e..e41ca1d58484 100644 --- a/security/smack/smack.h +++ b/security/smack/smack.h @@ -348,6 +348,7 @@ extern struct list_head smack_onlycap_list; #define SMACK_HASH_SLOTS 16 extern struct hlist_head smack_known_hash[SMACK_HASH_SLOTS]; +extern struct kmem_cache *smack_rule_cache; static inline struct task_smack *smack_cred(const struct cred *cred) { diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 5c1613519d5a..bd45c9139d34 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -59,6 +59,7 @@ DEFINE_MUTEX(smack_ipv6_lock); static LIST_HEAD(smk_ipv6_port_list); #endif static struct kmem_cache *smack_inode_cache; +struct kmem_cache *smack_rule_cache; int smack_enabled; #define A(s) {"smack"#s, sizeof("smack"#s) - 1, Opt_##s} @@ -354,7 +355,7 @@ static int smk_copy_rules(struct list_head *nhead, struct list_head *ohead, int rc = 0; list_for_each_entry_rcu(orp, ohead, list) { - nrp = kzalloc(sizeof(struct smack_rule), gfp); + nrp = kmem_cache_zalloc(smack_rule_cache, gfp); if (nrp == NULL) { rc = -ENOMEM; break; @@ -1931,7 +1932,7 @@ static void smack_cred_free(struct cred *cred) list_for_each_safe(l, n, &tsp->smk_rules) { rp = list_entry(l, struct smack_rule, list); list_del(&rp->list); - kfree(rp); + kmem_cache_free(smack_rule_cache, rp); } } @@ -4758,6 +4759,12 @@ static __init int smack_init(void) if (!smack_inode_cache) return -ENOMEM; + smack_rule_cache = KMEM_CACHE(smack_rule, 0); + if (!smack_rule_cache) { + kmem_cache_destroy(smack_inode_cache); + return -ENOMEM; + } + /* * Set the security state for the initial task. */ diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c index 8406738b45f2..47f73a0dabb1 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -236,7 +236,7 @@ static int smk_set_access(struct smack_parsed_rule *srp, } if (found == 0) { - sp = kzalloc(sizeof(*sp), GFP_KERNEL); + sp = kmem_cache_zalloc(smack_rule_cache, GFP_KERNEL); if (sp == NULL) { rc = -ENOMEM; goto out; -- cgit v1.2.3-58-ga151 From f7450bc6e76860564f3842a41892f9b74313cc23 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 3 Apr 2019 14:28:38 -0700 Subject: Smack: Fix IPv6 handling of 0 secmark Handle the case where the skb for an IPv6 packet contains a 0 in the secmark for a packet generated locally. This can only happen for system packets, so allow the access. Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index bd45c9139d34..b9abcdb36a73 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3907,6 +3907,8 @@ access_check: #ifdef SMACK_IPV6_SECMARK_LABELING if (skb && skb->secmark != 0) skp = smack_from_secid(skb->secmark); + else if (smk_ipv6_localhost(&sadd)) + break; else skp = smack_ipv6host_label(&sadd); if (skp == NULL) -- cgit v1.2.3-58-ga151 From b9ef5513c99bf9c8bfd9c9e8051b67f52b2dee1e Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Fri, 12 Apr 2019 19:59:35 +0900 Subject: smack: Check address length before reading address family KMSAN will complain if valid address length passed to bind()/connect()/ sendmsg() is shorter than sizeof("struct sockaddr"->sa_family) bytes. Also, since smk_ipv6_port_label()/smack_netlabel_send()/ smack_ipv6host_label()/smk_ipv6_check()/smk_ipv6_port_check() are not checking valid address length and/or address family, make sure we check both. The minimal valid length in smack_socket_connect() is changed from sizeof(struct sockaddr_in6) bytes to SIN6_LEN_RFC2133 bytes, for it seems that Smack is not using "struct sockaddr_in6"->sin6_scope_id field. Signed-off-by: Tetsuo Handa Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index b9abcdb36a73..b5b333d72637 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2806,13 +2806,17 @@ static int smack_socket_socketpair(struct socket *socka, * * Records the label bound to a port. * - * Returns 0 + * Returns 0 on success, and error code otherwise */ static int smack_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen) { - if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) + if (sock->sk != NULL && sock->sk->sk_family == PF_INET6) { + if (addrlen < SIN6_LEN_RFC2133 || + address->sa_family != AF_INET6) + return -EINVAL; smk_ipv6_port_label(sock, address); + } return 0; } #endif /* SMACK_IPV6_PORT_LABELING */ @@ -2848,12 +2852,13 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, switch (sock->sk->sk_family) { case PF_INET: - if (addrlen < sizeof(struct sockaddr_in)) + if (addrlen < sizeof(struct sockaddr_in) || + sap->sa_family != AF_INET) return -EINVAL; rc = smack_netlabel_send(sock->sk, (struct sockaddr_in *)sap); break; case PF_INET6: - if (addrlen < sizeof(struct sockaddr_in6)) + if (addrlen < SIN6_LEN_RFC2133 || sap->sa_family != AF_INET6) return -EINVAL; #ifdef SMACK_IPV6_SECMARK_LABELING rsp = smack_ipv6host_label(sip); @@ -3683,9 +3688,15 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, switch (sock->sk->sk_family) { case AF_INET: + if (msg->msg_namelen < sizeof(struct sockaddr_in) || + sip->sin_family != AF_INET) + return -EINVAL; rc = smack_netlabel_send(sock->sk, sip); break; case AF_INET6: + if (msg->msg_namelen < SIN6_LEN_RFC2133 || + sap->sin6_family != AF_INET6) + return -EINVAL; #ifdef SMACK_IPV6_SECMARK_LABELING rsp = smack_ipv6host_label(sap); if (rsp != NULL) -- cgit v1.2.3-58-ga151 From 619ae03e922b65a1a5d4269ceae1e9e13a058d6b Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Tue, 30 Apr 2019 14:13:32 -0700 Subject: Smack: Fix kbuild reported build error The variable sap is defined under ifdef, but a recently added use of the variable was not. Put that use under ifdef as well. Reported-by: kbuild test robot Signed-off-by: Casey Schaufler --- security/smack/smack_lsm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index b5b333d72637..0de725f88bed 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -3693,6 +3693,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, return -EINVAL; rc = smack_netlabel_send(sock->sk, sip); break; +#if IS_ENABLED(CONFIG_IPV6) case AF_INET6: if (msg->msg_namelen < SIN6_LEN_RFC2133 || sap->sin6_family != AF_INET6) @@ -3706,6 +3707,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, #ifdef SMACK_IPV6_PORT_LABELING rc = smk_ipv6_port_check(sock->sk, sap, SMK_SENDING); #endif +#endif /* IS_ENABLED(CONFIG_IPV6) */ break; } return rc; -- cgit v1.2.3-58-ga151