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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZMDzNSkRvvVsxUto@corigine.com>
Date: Wed, 26 Jul 2023 12:19:33 +0200
From: Simon Horman <simon.horman@...igine.com>
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>,
	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 v2 05/11] firewall: introduce stm32_firewall framework

On Tue, Jul 25, 2023 at 06:40:58PM +0200, Gatien Chevallier wrote:
> Introduce a STM32 firewall framework that offers to firewall consumers
> different firewall services such as the ability to check their access
> rights against their firewall controller(s).
> 
> The STM32 firewall framework offers a generic API for STM32 firewall
> controllers that is defined in their drivers to best fit the
> specificity of each firewall.
> 
> There are various types of firewalls:
> -Peripheral firewalls that filter accesses to peripherals
> -Memory firewalls that filter accesses to memories or memory regions
> -No type for undefined type of firewall
> 
> Signed-off-by: Gatien Chevallier <gatien.chevallier@...s.st.com>

...

> diff --git a/drivers/bus/stm32_firewall.c b/drivers/bus/stm32_firewall.c

...

> +int stm32_firewall_populate_bus(struct stm32_firewall_controller *firewall_controller)
> +{
> +	struct stm32_firewall *firewalls;
> +	struct device_node *child;
> +	struct device *parent;
> +	unsigned int i;
> +	int len;
> +	int err;
> +
> +	parent = firewall_controller->dev;
> +
> +	dev_dbg(parent, "Populating %s system bus\n", dev_name(firewall_controller->dev));
> +
> +	for_each_available_child_of_node(dev_of_node(parent), child) {
> +		/* The feature-domains property is mandatory for firewall bus devices */
> +		len = of_count_phandle_with_args(child, "feature-domains", "#feature-domain-cells");
> +		if (len <= 0)

Coccinelle says that, due to breaking out of a
for_each_available_child_of_node() loop, a call to of_node_put()
is needed here

> +			return -EINVAL;
> +
> +		firewalls = kcalloc(len, sizeof(*firewalls), GFP_KERNEL);
> +		if (!firewalls)

And here.

> +			return -ENOMEM;
> +
> +		err = stm32_firewall_get_firewall(child, firewalls, (unsigned int)len);
> +		if (err) {
> +			kfree(firewalls);

And here.

> +			return err;
> +		}
> +
> +		for (i = 0; i < len; i++) {
> +			if (firewall_controller->grant_access(firewall_controller,
> +							      firewalls[i].firewall_id)) {
> +				/*
> +				 * Peripheral access not allowed or not defined.
> +				 * Mark the node as populated so platform bus won't probe it
> +				 */
> +				of_node_set_flag(child, OF_POPULATED);
> +				dev_err(parent, "%s: Device driver will not be probed\n",
> +					child->full_name);
> +			}
> +		}
> +
> +		kfree(firewalls);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(stm32_firewall_populate_bus);

> diff --git a/drivers/bus/stm32_firewall.h b/drivers/bus/stm32_firewall.h

...

> +/**
> + * struct stm32_firewall_controller - Information on firewall controller supplying services
> + *
> + * @name			Name of the firewall controller

kernel-doc complains that name and the other fields of
struct stm32_firewall_controller are not documented.
I believe this is because a ':' is needed after the name of
the parameter (in this case 'name').

 * @name:			Name of the firewall controller

Likewise, elsewhere.

> + * @dev				Device reference of the firewall controller
> + * @mmio			Base address of the firewall controller
> + * @entry			List entry of the firewall controller list
> + * @type			Type of firewall
> + * @max_entries			Number of entries covered by the firewall
> + * @grant_access		Callback used to grant access for a device access against a
> + *				firewall controller
> + * @release_access		Callback used to release resources taken by a device when access was
> + *				granted
> + * @grant_memory_range_access	Callback used to grant access for a device to a given memory region
> + */
> +struct stm32_firewall_controller {
> +	const char *name;
> +	struct device *dev;
> +	void __iomem *mmio;
> +	struct list_head entry;
> +	unsigned int type;
> +	unsigned int max_entries;
> +
> +	int (*grant_access)(struct stm32_firewall_controller *ctrl, u32 id);
> +	void (*release_access)(struct stm32_firewall_controller *ctrl, u32 id);
> +	int (*grant_memory_range_access)(struct stm32_firewall_controller *ctrl, phys_addr_t paddr,
> +					 size_t size);
> +};
> +
> +/**
> + * int stm32_firewall_controller_register - Register a firewall controller to the STM32 firewall

kernel-doc seems unhappy about the presence of 'int' on this line.

 * stm32_firewall_controller_register - Register a firewall controller to the STM32 firewall

Likewise, elsewhere.

> + *					    framework
> + * @firewall_controller		Firewall controller to register
> + *
> + * Returns 0 in case of success or -ENODEV if no controller was given.
> + */
> +int stm32_firewall_controller_register(struct stm32_firewall_controller *firewall_controller);

...

> diff --git a/include/linux/bus/stm32_firewall_device.h b/include/linux/bus/stm32_firewall_device.h
> new file mode 100644
> index 000000000000..9bdc4060154c
> --- /dev/null
> +++ b/include/linux/bus/stm32_firewall_device.h
> @@ -0,0 +1,140 @@
> +/* 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;
> +
> +/**
> + * stm32_firewall - Information on a device's firewall. Each device can have more than one firewall.

kernel-doc seems unhappy about the absence of struct on this line.

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

...

-- 
pw-bot: changes-requested

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ