[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZNeSiFQGdOXbR+2S@vergenet.net>
Date: Sat, 12 Aug 2023 16:09:12 +0200
From: Simon Horman <horms@...nel.org>
To: Gatien Chevallier <gatien.chevallier@...s.st.com>
Cc: Oleksii_Moisieiev@...m.com, gregkh@...uxfoundation.org,
herbert@...dor.apana.org.au, davem@...emloft.net,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
conor+dt@...nel.org, alexandre.torgue@...s.st.com, vkoul@...nel.org,
jic23@...nel.org, olivier.moysan@...s.st.com,
arnaud.pouliquen@...s.st.com, mchehab@...nel.org,
fabrice.gasnier@...s.st.com, andi.shyti@...nel.org,
ulf.hansson@...aro.org, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, hugues.fruchet@...s.st.com, lee@...nel.org,
will@...nel.org, catalin.marinas@....com, arnd@...nel.org,
richardcochran@...il.com, Frank Rowand <frowand.list@...il.com>,
peng.fan@....nxp.com, linux-crypto@...r.kernel.org,
devicetree@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
dmaengine@...r.kernel.org, linux-i2c@...r.kernel.org,
linux-iio@...r.kernel.org, alsa-devel@...a-project.org,
linux-media@...r.kernel.org, linux-mmc@...r.kernel.org,
netdev@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-serial@...r.kernel.org, linux-spi@...r.kernel.org,
linux-usb@...r.kernel.org
Subject: Re: [PATCH v4 05/11] firewall: introduce stm32_firewall framework
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;
> +};
...
Powered by blists - more mailing lists