Messages in this thread | | | Date | Wed, 25 Oct 2023 17:15:25 -0700 | Subject | Re: [PATCH RFC v11 3/19] ipe: add evaluation loop | From | Fan Wu <> |
| |
On 10/23/2023 8:52 PM, Paul Moore wrote: > On Oct 4, 2023 Fan Wu <wufan@linux.microsoft.com> 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@linux.microsoft.com> >> Signed-off-by: Fan Wu <wufan@linux.microsoft.com> ... >> --- >> 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? > Yes for this part the unlock should be moved after the comparison. Thanks for spotting this.
>> + 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
| |