[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <75cbfaa1-ddfe-7f95-5b69-6e0c000668ec@schaufler-ca.com>
Date: Mon, 11 Mar 2019 14:05:14 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Vishal Goel <vishal.goel@...sung.com>,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: pankaj.m@...sung.com, a.sahrawat@...sung.com
Subject: Re: [PATCH 1/1] smack: removal of global rule list
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@...sung.com>
> Signed-off-by: Amit Sahrawat <a.sahrawat@...sung.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;
> }
Powered by blists - more mailing lists