On 6/28/21 8:40 AM, Ahmad Fatoum wrote:
> The Kconfig option already warns that the current behavior of
> machine_id_set_hashable() overriding previous calls can lead to the
> machine-id changing over updates. We don't yet have this problem in
> practice, because the only two upstream users are for bsec and ocotp,
> which are efuse drivers for different SoCs. On the other hand, users
> may want to get the unique ID from an EEPROM and with deep probe
> support, the initcall ordering will be independent of the actual probe
> order.
>
> Work around this issue by introducing a way for each board to explicitly
> reference a nvmem cell that should be hashed to produce the machine-id.
>
> If no such device tree property is supplied, the last call to
> machine_id_set_hashable() will be used as before.
>
> Cc: Bastian Krause <bst@xxxxxxxxxxxxxx>
> Cc: Uwe Kleine-König <ukl@xxxxxxxxxxxxxx>
> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
> ---
> common/Kconfig | 13 ++++++----
> common/machine_id.c | 41 ++++++++++++++++++++++++++++---
> drivers/nvmem/core.c | 44 ++++++++++++++++++++++++----------
> drivers/of/base.c | 11 +++++++++
> include/linux/nvmem-consumer.h | 5 ++++
> include/of.h | 6 +++++
> 6 files changed, 99 insertions(+), 21 deletions(-)
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 54f1eb633a32..a4a109f04f08 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1066,13 +1066,16 @@ config MACHINE_ID
> bool "compute unique machine-id"
> depends on FLEXIBLE_BOOTARGS
> depends on SHA1
> + select NVMEM
> help
> Compute a persistent machine-specific id and store it to $global.machine_id.
> - The id is a hash of device-specific information added via
> - machine_id_set_hashable(). If multiple sources are available, the
> - information provided by the last call prior to the late initcall
> - set_machine_id() is used to generate the machine id from. Thus when
> - updating barebox the machine id might change.
> + The id is a hash of device-specific information added either via
> + machine_id_set_hashable() or by hashing the nvmem cell referenced by the
> + /chosen/barebox,machine-id device tree property.
> +
> + With machine_id_set_hashable(), the last call prior to the late initcall
> + set_machine_id() willl be used to generate the machine id. This means
> + updating barebox may change the machine id!
>
> global.bootm.provide_machine_id may be used to automatically set
> the linux.bootargs.machine_id global variable with a value of
> diff --git a/common/machine_id.c b/common/machine_id.c
> index 6480806cd287..fd2f0888a6cd 100644
> --- a/common/machine_id.c
> +++ b/common/machine_id.c
> @@ -10,6 +10,9 @@
> #include <magicvar.h>
> #include <crypto/sha.h>
> #include <machine_id.h>
> +#include <linux/err.h>
> +#include <linux/nvmem-consumer.h>
> +#include <of.h>
>
> #define MACHINE_ID_LENGTH 32
>
> @@ -24,16 +27,49 @@ void machine_id_set_hashable(const void *hashable, size_t len)
> __machine_id_hashable_length = len;
> }
>
> +static const void *machine_id_get_hashable(size_t *len)
> +{
> + struct device_node *np = of_get_machineidpath();
> + struct nvmem_cell *cell;
> + void *ret;
> +
> + if (!np)
> + goto no_cell;
> +
> + cell = of_nvmem_cell_get_from_node(np);
> + if (IS_ERR(cell)) {
> + pr_err("Invalid barebox,machine-id-path: %pe\n", cell);
> + goto no_cell;
> + }
> +
> + ret = nvmem_cell_read(cell, len);
> + nvmem_cell_put(cell);
> + if (IS_ERR(ret)) {
> + pr_err("Couldn't read from barebox,machine-id-path: %pe\n", ret);
> + goto no_cell;
> + }
> +
> + return ret;
> +
> +no_cell:
> + *len = __machine_id_hashable_length;
> + return __machine_id_hashable;
> +}
> +
> static int machine_id_set_globalvar(void)
> {
> struct digest *digest = NULL;
> unsigned char machine_id[SHA1_DIGEST_SIZE];
> char hex_machine_id[MACHINE_ID_LENGTH];
> char *env_machine_id;
> + const void *hashable;
> + size_t length;
> int ret = 0;
>
> + hashable = machine_id_get_hashable(&length);
> +
> /* nothing to do if no hashable information provided */
> - if (!__machine_id_hashable)
> + if (!hashable)
> goto out;
>
> digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
> @@ -41,8 +77,7 @@ static int machine_id_set_globalvar(void)
> if (ret)
> goto out;
>
> - ret = digest_update(digest, __machine_id_hashable,
> - __machine_id_hashable_length);
> + ret = digest_update(digest, hashable, length);
> if (ret)
> goto out;
>
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index 4e558e165063..980304a8078b 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -361,29 +361,21 @@ static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>
> #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OFTREE)
> /**
> - * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + * of_nvmem_cell_get_from_node() - Get a nvmem cell from cell device node
> *
> - * @dev node: Device tree node that uses the nvmem cell
> - * @id: nvmem cell name from nvmem-cell-names property.
> + * @cell_np: Device tree node of the nvmem cell
> *
> * Return: Will be an ERR_PTR() on error or a valid pointer
> * to a struct nvmem_cell. The nvmem_cell will be freed by the
> * nvmem_cell_put().
> */
> -struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> - const char *name)
> +struct nvmem_cell *of_nvmem_cell_get_from_node(struct device_node *cell_np)
> {
> - struct device_node *cell_np, *nvmem_np;
> + struct device_node *nvmem_np;
> struct nvmem_cell *cell;
> struct nvmem_device *nvmem;
> const __be32 *addr;
> - int rval, len, index;
> -
> - index = of_property_match_string(np, "nvmem-cell-names", name);
> -
> - cell_np = of_parse_phandle(np, "nvmem-cells", index);
> - if (!cell_np)
> - return ERR_PTR(-EINVAL);
> + int rval, len;
>
> nvmem_np = of_get_parent(cell_np);
> if (!nvmem_np)
> @@ -445,6 +437,32 @@ err_mem:
>
> return ERR_PTR(rval);
> }
> +EXPORT_SYMBOL_GPL(of_nvmem_cell_get_from_node);
> +
> +/**
> + * of_nvmem_cell_get() - Get a nvmem cell from given device node and cell id
> + *
> + * @dev node: Device tree node that uses the nvmem cell
> + * @id: nvmem cell name from nvmem-cell-names property.
> + *
> + * Return: Will be an ERR_PTR() on error or a valid pointer
> + * to a struct nvmem_cell. The nvmem_cell will be freed by the
> + * nvmem_cell_put().
> + */
> +struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> + const char *name)
> +{
> + struct device_node *cell_np;
> + int index;
> +
> + index = of_property_match_string(np, "nvmem-cell-names", name);
> +
> + cell_np = of_parse_phandle(np, "nvmem-cells", index);
> + if (!cell_np)
> + return ERR_PTR(-EINVAL);
> +
> + return of_nvmem_cell_get_from_node(cell_np);
> +}
> EXPORT_SYMBOL_GPL(of_nvmem_cell_get);
> #endif
Shouldn't these changes be part of a separate patch?
Regards,
Bastian
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 4104d7589305..a5b74707fc4f 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -2525,6 +2525,17 @@ int of_device_is_stdout_path(struct device_d *dev, unsigned int *baudrate)
> return true;
> }
>
> +struct device_node *of_get_machineidpath(void)
> +{
> + const char *name;
> +
> + name = of_get_property(of_chosen, "barebox,machine-id-path", NULL);
> + if (!name)
> + return NULL;
> +
> + return of_find_node_by_path_or_alias(NULL, name);
> +}
> +
> /**
> * of_add_initrd - add initrd properties to the devicetree
> * @root - the root node of the tree
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index b979f23372a6..639a7ebbabae 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -122,11 +122,16 @@ static inline int nvmem_device_write(struct nvmem_device *nvmem,
> #endif /* CONFIG_NVMEM */
>
> #if IS_ENABLED(CONFIG_NVMEM) && IS_ENABLED(CONFIG_OFTREE)
> +struct nvmem_cell *of_nvmem_cell_get_from_node(struct device_node *cell_np);
> struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> const char *name);
> struct nvmem_device *of_nvmem_device_get(struct device_node *np,
> const char *name);
> #else
> +static inline struct nvmem_cell *of_nvmem_cell_get_from_node(struct device_node *cell_np)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> static inline struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
> const char *name)
> {
> diff --git a/include/of.h b/include/of.h
> index d82790c0523e..677f48d0aba1 100644
> --- a/include/of.h
> +++ b/include/of.h
> @@ -290,6 +290,7 @@ int of_fixup_partitions(struct device_node *np, struct cdev *cdev);
> int of_partitions_register_fixup(struct cdev *cdev);
> struct device_node *of_get_stdoutpath(unsigned int *);
> int of_device_is_stdout_path(struct device_d *dev, unsigned int *baudrate);
> +struct device_node *of_get_machineidpath(void);
> const char *of_get_model(void);
> void *of_flatten_dtb(struct device_node *node);
> int of_add_memory(struct device_node *node, bool dump);
> @@ -336,6 +337,11 @@ static inline int of_device_is_stdout_path(struct device_d *dev, unsigned int *b
> return 0;
> }
>
> +static inline struct device_node *of_get_machineidpath(void)
> +{
> + return NULL;
> +}
> +
> static inline const char *of_get_model(void)
> {
> return NULL;
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
