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.
> +
> + /* 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;
> +};
...
