| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3c232c26-e03c-31ec-8ec8-f99c86a3ab83@foss.st.com>
Date: Thu, 28 Sep 2023 17:43:35 +0200
From: Gatien CHEVALLIER <gatien.chevallier@...s.st.com>
To: Simon Horman <horms@...nel.org>
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 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;
>> +};
>
> ...
Powered by blists - more mailing lists