On 10/23/2023 8:52 PM, Paul Moore wrote:
On Oct 4, 2023 Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:IPE must have a centralized function to evaluate incoming callers against IPE's policy. This iteration of the policy for against the rules for that specific caller is known as the evaluation loop. Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx> Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
...
Yes for this part the unlock should be moved after the comparison. Thanks for spotting this.--- security/ipe/Makefile | 1 + security/ipe/eval.c | 96 +++++++++++++++++++++++++++++++++++++++++++ security/ipe/eval.h | 24 +++++++++++ 3 files changed, 121 insertions(+) create mode 100644 security/ipe/eval.c create mode 100644 security/ipe/eval.h...diff --git a/security/ipe/eval.c b/security/ipe/eval.c new file mode 100644 index 000000000000..5533c359bbeb --- /dev/null +++ b/security/ipe/eval.c @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) Microsoft Corporation. All rights reserved. + */ + +#include <linux/fs.h> +#include <linux/types.h> +#include <linux/slab.h> +#include <linux/file.h> +#include <linux/sched.h> +#include <linux/rcupdate.h> + +#include "ipe.h" +#include "eval.h" +#include "policy.h" + +struct ipe_policy __rcu *ipe_active_policy; + +/** + * evaluate_property - Analyze @ctx against a property. + * @ctx: Supplies a pointer to the context to be evaluated. + * @p: Supplies a pointer to the property to be evaluated. + * + * Return: + * * true - The current @ctx match the @p + * * false - The current @ctx doesn't match the @p + */ +static bool evaluate_property(const struct ipe_eval_ctx *const ctx, + struct ipe_prop *p) +{ + return false; +} + +/** + * ipe_evaluate_event - Analyze @ctx against the current active policy. + * @ctx: Supplies a pointer to the context to be evaluated. + * + * This is the loop where all policy evaluation happens against IPE policy. + * + * Return: + * * 0 - OK + * * -EACCES - @ctx did not pass evaluation. + * * !0 - Error + */ +int ipe_evaluate_event(const struct ipe_eval_ctx *const ctx) +{ + bool match = false; + enum ipe_action_type action; + struct ipe_policy *pol = NULL; + const struct ipe_rule *rule = NULL; + const struct ipe_op_table *rules = NULL; + struct ipe_prop *prop = NULL; + + rcu_read_lock(); + + pol = rcu_dereference(ipe_active_policy); + if (!pol) { + rcu_read_unlock(); + return 0; + } + + if (ctx->op == IPE_OP_INVALID) { + rcu_read_unlock(); + if (pol->parsed->global_default_action == IPE_ACTION_DENY) + return -EACCES;Assuming that the RCU lock protects @pol, shouldn't it be held until after the global_default_action comparison?
+ return 0; + } + + rules = &pol->parsed->rules[ctx->op]; + + list_for_each_entry(rule, &rules->rules, next) { + match = true; + + list_for_each_entry(prop, &rule->props, next) { + match = match && evaluate_property(ctx, prop);The @match variable will always be true on the right side above, or am I missing something?
Yes the "match &&" are completely unnecessary. I will remove them. -Fan
+ if (!match) + break; + } + + if (match) + break; + } + + if (match) + action = rule->action; + else if (rules->default_action != IPE_ACTION_INVALID) + action = rules->default_action; + else + action = pol->parsed->global_default_action; + + rcu_read_unlock(); + if (action == IPE_ACTION_DENY) + return -EACCES; + + return 0; +}-- paul-moore.com
|