On 8/12/23 16:09, Simon Horman wrote:
On Fri, Aug 11, 2023 at 12:07:25PM +0200, Gatien Chevallier wrote:
...
diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c new file mode 100644 index 000000000000..900f3b052a66 --- /dev/null +++ b/drivers/bus/stm32_firewall.c @@ -0,0 +1,293 @@ +// SPDX-License-Identifier: GPL-2.0-only +/*
- Copyright (C) 2023, STMicroelectronics - All Rights Reserved
- */
+#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/bus/stm32_firewall_device.h> +#include <linux/device.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_platform.h> +#include <linux/platform_device.h> +#include <linux/types.h> +#include <linux/slab.h>
+#include "stm32_firewall.h"
+/* Corresponds to STM32_FIREWALL_MAX_EXTRA_ARGS + firewall ID */ +#define STM32_FIREWALL_MAX_ARGS (STM32_FIREWALL_MAX_EXTRA_ARGS + 1)
+static LIST_HEAD(firewall_controller_list); +static DEFINE_MUTEX(firewall_controller_list_lock);
+/* Firewall device API */ +int stm32_firewall_get_firewall(struct device_node *np, struct stm32_firewall *firewall,
unsigned int nb_firewall)+{
- struct stm32_firewall_controller *ctrl;
- struct of_phandle_iterator it;
- unsigned int i, j = 0;
- int err;
- if (!firewall || !nb_firewall)
return -EINVAL;- /* Parse property with phandle parsed out */
- of_for_each_phandle(&it, err, np, "feature-domains", "#feature-domain-cells", 0) {
struct of_phandle_args provider_args;struct device_node *provider = it.node;const char *fw_entry;bool match = false;if (err) {pr_err("Unable to get feature-domains property for node %s\n, err: %d",np->full_name, err);of_node_put(provider);return err;}if (j > nb_firewall) {pr_err("Too many firewall controllers");of_node_put(provider);return -EINVAL;}provider_args.args_count = of_phandle_iterator_args(&it, provider_args.args,STM32_FIREWALL_MAX_ARGS);/* Check if the parsed phandle corresponds to a registered firewall controller */mutex_lock(&firewall_controller_list_lock);list_for_each_entry(ctrl, &firewall_controller_list, entry) {if (ctrl->dev->of_node->phandle == it.phandle) {match = true;firewall[j].firewall_ctrl = ctrl;break;}}mutex_unlock(&firewall_controller_list_lock);if (!match) {firewall[j].firewall_ctrl = NULL;pr_err("No firewall controller registered for %s\n", np->full_name);of_node_put(provider);return -ENODEV;}err = of_property_read_string_index(np, "feature-domain-names", j, &fw_entry);if (err == 0)firewall[j].entry = fw_entry;/* Handle the case when there are no arguments given along with the phandle */if (provider_args.args_count < 0 ||provider_args.args_count > STM32_FIREWALL_MAX_ARGS) {of_node_put(provider);return -EINVAL;} else if (provider_args.args_count == 0) {firewall[j].extra_args_size = 0;firewall[j].firewall_id = U32_MAX;j++;continue;}/* The firewall ID is always the first argument */firewall[j].firewall_id = provider_args.args[0];/* Extra args start at the third argument */for (i = 0; i < provider_args.args_count; i++)firewall[j].extra_args[i] = provider_args.args[i + 1];Hi Gatien,
Above it is checked that the maximum value of provider_args.args_count is STM32_FIREWALL_MAX_ARGS. So here the maximum value of i is STM32_FIREWALL_MAX_ARGS - 1.
STM32_FIREWALL_MAX_ARGS is defined as STM32_FIREWALL_MAX_EXTRA_ARGS + 1 And STM32_FIREWALL_MAX_EXTRA_ARGS is defined as 5. So the maximum value of i is (5 + 1 - 1) = 5.
firewall[j] is of type struct stm32_firewall. And its args field has STM32_FIREWALL_MAX_EXTRA_ARGS (5) elements. Thus the maximum valid index is (5 - 1) = 4.
But the line above may access index 5.
Flagged by Smatch.
Hi Simon,
Thank you for pointing this out.
I'll correct it for V5.
Best regards, Gatien
/* Remove the firewall ID arg that is not an extra argument */firewall[j].extra_args_size = provider_args.args_count - 1;j++;- }
- return 0;
+} +EXPORT_SYMBOL_GPL(stm32_firewall_get_firewall);
...
diff --git a/include/linux/bus/stm32_firewall_device.h b/include/linux/bus/stm32_firewall_device.h new file mode 100644 index 000000000000..7b4450a8ec15 --- /dev/null +++ b/include/linux/bus/stm32_firewall_device.h @@ -0,0 +1,141 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/*
- Copyright (C) 2023, STMicroelectronics - All Rights Reserved
- */
+#ifndef STM32_FIREWALL_DEVICE_H +#define STM32_FIREWALL_DEVICE_H
+#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/types.h>
+#define STM32_FIREWALL_MAX_EXTRA_ARGS 5
+/* Opaque reference to stm32_firewall_controller */ +struct stm32_firewall_controller;
+/**
- struct stm32_firewall - Information on a device's firewall. Each device can have more than one
firewall.
- @firewall_ctrl: Pointer referencing a firewall controller of the device. It is
opaque so a device cannot manipulate the controller's ops or access
the controller's data
- @extra_args: Extra arguments that are implementation dependent
- @entry: Name of the firewall entry
- @extra_args_size: Number of extra arguments
- @firewall_id: Firewall ID associated the device for this firewall controller
- */
+struct stm32_firewall {
- struct stm32_firewall_controller *firewall_ctrl;
- u32 extra_args[STM32_FIREWALL_MAX_EXTRA_ARGS];
- const char *entry;
- size_t extra_args_size;
- u32 firewall_id;
+};
...