On Mon, 15 Jul 2024, at 13:54, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@xxxxxxxxxx> > > diff --git a/drivers/nvmem/layouts/u-boot-env.c > b/drivers/nvmem/layouts/u-boot-env.c > new file mode 100644 > index 000000000000..5217dc4a52f8 > --- /dev/null > +++ b/drivers/nvmem/layouts/u-boot-env.c > @@ -0,0 +1,203 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 - 2023 Rafał Miłecki <rafal@xxxxxxxxxx> > + */ > + > +#include <linux/crc32.h> > +#include <linux/etherdevice.h> > +#include <linux/export.h> > +#include <linux/if_ether.h> > +#include <linux/nvmem-consumer.h> > +#include <linux/nvmem-provider.h> > +#include <linux/of.h> > +#include <linux/slab.h> > + > +#include "u-boot-env.h" > + > +struct u_boot_env_image_single { > + __le32 crc32; > + uint8_t data[]; > +} __packed; > + > +struct u_boot_env_image_redundant { > + __le32 crc32; > + u8 mark; > + uint8_t data[]; > +} __packed; > + > +struct u_boot_env_image_broadcom { > + __le32 magic; > + __le32 len; > + __le32 crc32; > + DECLARE_FLEX_ARRAY(uint8_t, data); > +} __packed; > + > +static int u_boot_env_read_post_process_ethaddr(void *context, const > char *id, int index, > + unsigned int offset, void *buf, size_t bytes) > +{ > + u8 mac[ETH_ALEN]; > + > + if (bytes != 3 * ETH_ALEN - 1) > + return -EINVAL; > + > + if (!mac_pton(buf, mac)) > + return -EINVAL; > + > + if (index) > + eth_addr_add(mac, index); > + > + ether_addr_copy(buf, mac); > + > + return 0; > +} > + > +static int u_boot_env_parse_cells(struct device *dev, struct > nvmem_device *nvmem, uint8_t *buf, > + size_t data_offset, size_t data_len) > +{ > + char *data = buf + data_offset; > + char *var, *value, *eq; > + > + for (var = data; > + var < data + data_len && *var; > + var = value + strlen(value) + 1) { > + struct nvmem_cell_info info = {}; > + > + eq = strchr(var, '='); > + if (!eq) > + break; > + *eq = '\0'; > + value = eq + 1; > + > + info.name = devm_kstrdup(dev, var, GFP_KERNEL); > + if (!info.name) > + return -ENOMEM; > + info.offset = data_offset + value - data; > + info.bytes = strlen(value); > + info.np = of_get_child_by_name(dev->of_node, info.name); > + if (!strcmp(var, "ethaddr")) { > + info.raw_len = strlen(value); > + info.bytes = ETH_ALEN; > + info.read_post_process = u_boot_env_read_post_process_ethaddr; > + } > + > + nvmem_add_one_cell(nvmem, &info); > + } > + > + return 0; > +} > + > +int u_boot_env_parse(struct device *dev, struct nvmem_device *nvmem, > + enum u_boot_env_format format) > +{ > + size_t crc32_data_offset; > + size_t crc32_data_len; > + size_t crc32_offset; > + __le32 *crc32_addr; > + size_t data_offset; > + size_t data_len; > + size_t dev_size; > + uint32_t crc32; > + uint32_t calc; > + uint8_t *buf; > + int bytes; > + int err; > + > + dev_size = nvmem_dev_size(nvmem); > + > + buf = kzalloc(dev_size, GFP_KERNEL); > + if (!buf) { > + err = -ENOMEM; > + goto err_out; > + } > + > + bytes = nvmem_device_read(nvmem, 0, dev_size, buf); > + if (bytes < 0) { > + err = bytes; > + goto err_kfree; > + } else if (bytes != dev_size) { > + err = -EIO; > + goto err_kfree; > + } > + > + switch (format) { > + case U_BOOT_FORMAT_SINGLE: > + crc32_offset = offsetof(struct u_boot_env_image_single, crc32); > + crc32_data_offset = offsetof(struct u_boot_env_image_single, data); > + data_offset = offsetof(struct u_boot_env_image_single, data); > + break; > + case U_BOOT_FORMAT_REDUNDANT: > + crc32_offset = offsetof(struct u_boot_env_image_redundant, crc32); > + crc32_data_offset = offsetof(struct u_boot_env_image_redundant, > data); > + data_offset = offsetof(struct u_boot_env_image_redundant, data); > + break; > + case U_BOOT_FORMAT_BROADCOM: > + crc32_offset = offsetof(struct u_boot_env_image_broadcom, crc32); > + crc32_data_offset = offsetof(struct u_boot_env_image_broadcom, data); > + data_offset = offsetof(struct u_boot_env_image_broadcom, data); > + break; > + } Could we error somewhere if bytes or dev_size is < crc32_data_offset or alternatively if bytes is zero (above, beside bytes != dev_size)? Detailed here: https://lore.kernel.org/lkml/20240612031510.14414-1-git@xxxxxxxxxxxxxxxxxxxxxxxxxxx/ mtd partitioning can deliver a zero length partition (if misconfigured: Example u-boot partition starts beyond hardware NOR length). > + crc32_addr = (__le32 *)(buf + crc32_offset); > + crc32 = le32_to_cpu(*crc32_addr); > + crc32_data_len = dev_size - crc32_data_offset; > + data_len = dev_size - data_offset; > + > + calc = crc32(~0, buf + crc32_data_offset, crc32_data_len) ^ ~0L; > + if (calc != crc32) { > + dev_err(dev, "Invalid calculated CRC32: 0x%08x (expected: > 0x%08x)\n", calc, crc32); > + err = -EINVAL; > + goto err_kfree; > + } > + > + buf[dev_size - 1] = '\0'; > + err = u_boot_env_parse_cells(dev, nvmem, buf, data_offset, data_len); > + > +err_kfree: > + kfree(buf); > +err_out: > + return err; > +} > +EXPORT_SYMBOL_GPL(u_boot_env_parse); > + > +static int u_boot_env_add_cells(struct nvmem_layout *layout) > +{ > + struct device *dev = &layout->dev; > + enum u_boot_env_format format; > + > + format = (uintptr_t)device_get_match_data(dev); > + > + return u_boot_env_parse(dev, layout->nvmem, format); > +} > + > +static int u_boot_env_probe(struct nvmem_layout *layout) > +{ > + layout->add_cells = u_boot_env_add_cells; > + > + return nvmem_layout_register(layout); > +} > + > +static void u_boot_env_remove(struct nvmem_layout *layout) > +{ > + nvmem_layout_unregister(layout); > +} > + > +static const struct of_device_id u_boot_env_of_match_table[] = { > + { .compatible = "u-boot,env", .data = (void *)U_BOOT_FORMAT_SINGLE, }, > + { .compatible = "u-boot,env-redundant-bool", .data = (void > *)U_BOOT_FORMAT_REDUNDANT, }, > + { .compatible = "u-boot,env-redundant-count", .data = (void > *)U_BOOT_FORMAT_REDUNDANT, }, > + { .compatible = "brcm,env", .data = (void *)U_BOOT_FORMAT_BROADCOM, }, > + {}, > +}; > + > +static struct nvmem_layout_driver u_boot_env_layout = { > + .driver = { > + .name = "u-boot-env-layout", > + .of_match_table = u_boot_env_of_match_table, > + }, > + .probe = u_boot_env_probe, > + .remove = u_boot_env_remove, > +}; > +module_nvmem_layout_driver(u_boot_env_layout); > + > +MODULE_AUTHOR("Rafał Miłecki"); > +MODULE_LICENSE("GPL"); > +MODULE_DEVICE_TABLE(of, u_boot_env_of_match_table); -- John Thomson