linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] smack: removal of global rule list
       [not found] <CGME20190307113608epcas5p1be102a5a4592055ab3c97df8a8339d98@epcas5p1.samsung.com>
@ 2019-03-07 11:25 ` Vishal Goel
  2019-03-11 21:05   ` Casey Schaufler
  0 siblings, 1 reply; 2+ messages in thread
From: Vishal Goel @ 2019-03-07 11:25 UTC (permalink / raw)
  To: casey, linux-security-module, linux-kernel
  Cc: pankaj.m, a.sahrawat, Vishal Goel

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 <vishal.goel@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
 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 f6482e5..2a8a1f5 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 @@ enum smk_inos {
 
 /*
  * 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 ssize_t smk_write_access(struct file *file, const char __user *buf,
 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;
 }
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 1/1] smack: removal of global rule list
  2019-03-07 11:25 ` [PATCH 1/1] smack: removal of global rule list Vishal Goel
@ 2019-03-11 21:05   ` Casey Schaufler
  0 siblings, 0 replies; 2+ messages in thread
From: Casey Schaufler @ 2019-03-11 21:05 UTC (permalink / raw)
  To: Vishal Goel, linux-security-module, linux-kernel; +Cc: pankaj.m, a.sahrawat

On 3/7/2019 3:25 AM, Vishal Goel wrote:
> 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 <vishal.goel@samsung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>

Looks fine. I will take it for 5.2.

> ---
>   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 f6482e5..2a8a1f5 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 @@ enum smk_inos {
>   
>   /*
>    * 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 ssize_t smk_write_access(struct file *file, const char __user *buf,
>   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;
>   }

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-03-11 21:05 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20190307113608epcas5p1be102a5a4592055ab3c97df8a8339d98@epcas5p1.samsung.com>
2019-03-07 11:25 ` [PATCH 1/1] smack: removal of global rule list Vishal Goel
2019-03-11 21:05   ` Casey Schaufler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).