Re: [PATCH v20 02/20] ipe: add policy parser

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/10/2024 8:50 AM, Serge E. Hallyn wrote:
On Fri, Aug 02, 2024 at 11:08:16PM -0700, Fan Wu wrote:
From: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>

IPE's interpretation of the what the user trusts is accomplished through

nit: "of what the user trusts" (drop the extra 'the')

its policy. IPE's design is to not provide support for a single trust
provider, but to support multiple providers to enable the end-user to
choose the best one to seek their needs.

This requires the policy to be rather flexible and modular so that
integrity providers, like fs-verity, dm-verity, or some other system,
can plug into the policy with minimal code changes.

Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>

This all looks fine.  Just one comment below.

Thank you for reviewing this!


+/**
+ * parse_rule() - parse a policy rule line.
+ * @line: Supplies rule line to be parsed.
+ * @p: Supplies the partial parsed policy.
+ *
+ * Return:
+ * * 0		- Success
+ * * %-ENOMEM	- Out of memory (OOM)
+ * * %-EBADMSG	- Policy syntax error
+ */
+static int parse_rule(char *line, struct ipe_parsed_policy *p)
+{
+	enum ipe_action_type action = IPE_ACTION_INVALID;
+	enum ipe_op_type op = IPE_OP_INVALID;
+	bool is_default_rule = false;
+	struct ipe_rule *r = NULL;
+	bool first_token = true;
+	bool op_parsed = false;
+	int rc = 0;
+	char *t;
+
+	r = kzalloc(sizeof(*r), GFP_KERNEL);
+	if (!r)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&r->next);
+	INIT_LIST_HEAD(&r->props);
+
+	while (t = strsep(&line, IPE_POLICY_DELIM), line) {

If line is passed in as NULL, t will be NULL on the first test.  Then
you'll break out and call parse_action(NULL), which calls
match_token(NULL, ...), which I do not think is safe.

I realize the current caller won't pass in NULL, but it seems worth
checking for here in case some future caller is added by someone
who's unaware.

Or, maybe add 'line must not be null' to the function description.


Yes, I agree that adding a NULL check would be better. I will include it in the next version.

-Fan




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux