| 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: <8a8653c5-b112-4042-cbdf-8498e38d14ee@linaro.org>
Date: Mon, 1 Jun 2020 09:50:52 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Vadym Kochan <vadym.kochan@...ision.eu>,
linux-kernel@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Taras Chornyi <taras.chornyi@...ision.eu>
Subject: Re: [PATCH v2 2/2] nvmem: add ONIE NVMEM cells support
On 30/05/2020 00:04, Vadym Kochan wrote:
> ONIE is a small operating system, pre-installed on bare metal network
> switches, that provides an environment for automated provisioning.
>
> This system requires that NVMEM (EEPROM) device holds various system
> information (mac address, platform name, etc) in a special TLV layout.
>
> The driver registers ONIE TLV attributes as NVMEM cells which can be
> accessed by other platform driver. Also it allows to use
> of_get_mac_address() to retrieve mac address for the netdev.
>
> Signed-off-by: Vadym Kochan <vadym.kochan@...ision.eu>
> ---
> drivers/nvmem/Kconfig | 9 +
> drivers/nvmem/Makefile | 3 +
> drivers/nvmem/onie-cells.c | 332 +++++++++++++++++++++++++++++++++++++
Is there a reason why Device tree bindings are missing for this driver?
> 3 files changed, 344 insertions(+)
> create mode 100644 drivers/nvmem/onie-cells.c
>
> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
> index d7b7f6d688e7..dd9298487992 100644
> --- a/drivers/nvmem/Kconfig
> +++ b/drivers/nvmem/Kconfig
> @@ -273,4 +273,13 @@ config SPRD_EFUSE
> This driver can also be built as a module. If so, the module
> will be called nvmem-sprd-efuse.
>
> +config NVMEM_ONIE_CELLS
> + tristate "ONIE TLV cells support"
> + help
> + This is a driver to provide cells from ONIE TLV structure stored
> + on NVME device.
> +
> + This driver can also be built as a module. If so, the module
> + will be called nvmem-onie-cells.
> +
> endif
> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
> index a7c377218341..2199784a489f 100644
> --- a/drivers/nvmem/Makefile
> +++ b/drivers/nvmem/Makefile
> @@ -55,3 +55,6 @@ obj-$(CONFIG_NVMEM_ZYNQMP) += nvmem_zynqmp_nvmem.o
> nvmem_zynqmp_nvmem-y := zynqmp_nvmem.o
> obj-$(CONFIG_SPRD_EFUSE) += nvmem_sprd_efuse.o
> nvmem_sprd_efuse-y := sprd-efuse.o
> +
> +obj-$(CONFIG_NVMEM_ONIE_CELLS) += nvmem-onie-cells.o
> +nvmem-onie-cells-y := onie-cells.o
> diff --git a/drivers/nvmem/onie-cells.c b/drivers/nvmem/onie-cells.c
> new file mode 100644
> index 000000000000..1e8b4b8d1c0d
> --- /dev/null
> +++ b/drivers/nvmem/onie-cells.c
> @@ -0,0 +1,332 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ONIE NVMEM cells provider
> + *
> + * Author: Vadym Kochan <vadym.kochan@...ision.eu>
> + */
> +
> +#define ONIE_NVMEM_DRVNAME "onie-nvmem-cells"
> +
> +#define pr_fmt(fmt) ONIE_NVMEM_DRVNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/nvmem-provider.h>
> +
> +#define ONIE_NVMEM_TLV_MAX_LEN 2048
> +
> +#define ONIE_NVMEM_HDR_ID "TlvInfo"
> +
...
> +
> +static int onie_nvmem_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct nvmem_cell_info *cells;
> + struct onie_nvmem_attr *attr;
> + struct nvmem_device *nvmem;
> + struct onie_nvmem *onie;
> + unsigned int ncells = 0;
> + int err;
> +
> + nvmem = of_nvmem_device_get(np, NULL);
> + if (IS_ERR(nvmem))
> + return PTR_ERR(nvmem);
> +
TBH, this looks completely incorrect way to do this and misuse of nvmem
consumer interface.
Ideally nvmem provider driver should populate "cells" in struct
nvmem_config after decoding them and then register nvmem provider.
That should just work!
--srini
> + onie = kmalloc(sizeof(*onie), GFP_KERNEL);
> + if (!onie) {
> + err = -ENOMEM;
> + goto err_nvmem_alloc;
> + }
> +
> + INIT_LIST_HEAD(&onie->attrs);
> + onie->attr_count = 0;
> + onie->nvmem = nvmem;
> +
> + err = onie_nvmem_decode(onie);
> + if (err)
> + goto err_nvmem_decode;
> +
> + if (!onie->attr_count) {
> + pr_err("%s: has no ONIE attributes\n", nvmem_dev_name(nvmem));
> + err = -EINVAL;
> + goto err_no_attrs;
> + }
> +
> + cells = kmalloc_array(onie->attr_count, sizeof(*cells), GFP_KERNEL);
> + if (!cells) {
> + err = -ENOMEM;
> + goto err_cells_alloc;
> + }
> +
> + onie->cell_lookup = kmalloc_array(onie->attr_count,
> + sizeof(struct nvmem_cell_lookup),
> + GFP_KERNEL);
> + if (!onie->cell_lookup) {
> + err = -ENOMEM;
> + goto err_lookup_alloc;
> + }
> +
> + list_for_each_entry(attr, &onie->attrs, head) {
> + struct nvmem_cell_lookup *lookup;
> + struct nvmem_cell_info *cell;
> +
> + cell = &cells[ncells];
> +
> + lookup = &onie->cell_lookup[ncells];
> + lookup->con_id = NULL;
> +
> + cell->offset = attr->offset;
> + cell->name = attr->name;
> + cell->bytes = attr->len;
> + cell->bit_offset = 0;
> + cell->nbits = 0;
> +
> + lookup->nvmem_name = nvmem_dev_name(onie->nvmem);
> + lookup->dev_id = dev_name(dev);
> + lookup->cell_name = cell->name;
> + lookup->con_id = cell->name;
> +
> + ncells++;
> + }
> +
> + onie->cell_tbl.nvmem_name = nvmem_dev_name(onie->nvmem);
> + onie->cell_tbl.ncells = ncells;
> + onie->cell_tbl.cells = cells;
> +
> + nvmem_add_cell_table(&onie->cell_tbl);
> + nvmem_add_cell_lookups(onie->cell_lookup, ncells);
> +
> + dev_set_drvdata(dev, onie);
> +
> + onie_nvmem_attrs_free(onie);
> +
> + nvmem_device_put(nvmem);
> +
> + return 0;
> +
> +err_lookup_alloc:
> + kfree(onie->cell_tbl.cells);
> +err_cells_alloc:
> + onie_nvmem_attrs_free(onie);
> +err_no_attrs:
> +err_nvmem_decode:
> + kfree(onie);
> +err_nvmem_alloc:
> + nvmem_device_put(nvmem);
> +
> + return err;
> +}
> +
Powered by blists - more mailing lists