Re: [PATCH 2/3] common: machine_id: introduce machine id generation and pass id on

[Sascha Hauer <s.hauer@pengutronix.de>]

On Tue, Jul 16, 2019 at 12:58:36PM +0200, Bastian Krause wrote:
> By default systemd generates a machine id on first boot and tries to
> persist it (see `man machine-id`). When the root file system is read-only
> systemd cannot persist the machine id. In case multiple redundant slots
> are used the machine id will vary. When not handled explicitly the
> machine id will also change during updates.
> 
> It is possible to pass a machine id to the kernel which will be used by
> systemd (systemd.machine_id=).
> 
> This adds functionality to pass device-specific information that will be
> hashed to generate a persistent unique machine id. The machine id will
> be finally added to the kernel parameters via the
> linux.bootargs.machine_id global variable.
> 
> Note: if multiple sources provide hashable device-specific information
> (via machine_id_set_hashable()) 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.
> 
> Signed-off-by: Bastian Krause <bst@pengutronix.de>
> ---
>  common/Kconfig       | 11 ++++++++
>  common/Makefile      |  1 +
>  common/machine_id.c  | 65 ++++++++++++++++++++++++++++++++++++++++++++
>  include/machine_id.h |  6 ++++
>  4 files changed, 83 insertions(+)
>  create mode 100644 common/machine_id.c
>  create mode 100644 include/machine_id.h
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 8aad5baecd..4b2d79350d 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -982,6 +982,17 @@ config RESET_SOURCE
>  	  of the reset and why the bootloader is currently running. It can be
>  	  useful for any kind of system recovery or repair.
>  
> +config MACHINE_ID
> +	bool "pass machine-id to kernel"
> +	depends on FLEXIBLE_BOOTARGS
> +	select DIGEST
> +	select DIGEST_SHA1_GENERIC
> +	help
> +	  Sets the linux.bootargs.machine_id global variable with a value of
> +	  systemd.machine_id=UID. The UID is a persistent device-specific
> +	  id. It is a hash over device-specific information provided by various
> +	  sources.
> +
>  endmenu
>  
>  menu "Debugging"
> diff --git a/common/Makefile b/common/Makefile
> index a284655fc1..10960169f9 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -11,6 +11,7 @@ obj-y				+= bootsource.o
>  obj-$(CONFIG_ELF)		+= elf.o
>  obj-y				+= restart.o
>  obj-y				+= poweroff.o
> +obj-$(CONFIG_MACHINE_ID)	+= machine_id.o
>  obj-$(CONFIG_AUTO_COMPLETE)	+= complete.o
>  obj-y				+= version.o
>  obj-$(CONFIG_BAREBOX_UPDATE)	+= bbu.o
> diff --git a/common/machine_id.c b/common/machine_id.c
> new file mode 100644
> index 0000000000..54c1820086
> --- /dev/null
> +++ b/common/machine_id.c
> @@ -0,0 +1,65 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 Pengutronix, Bastian Krause <kernel@pengutronix.de>
> + */
> +
> +#define pr_fmt(fmt) "machine-id: " fmt
> +
> +#include <common.h>
> +#include <init.h>
> +#include <digest.h>
> +#include <globalvar.h>
> +#include <crypto/sha.h>
> +#include <machine_id.h>
> +
> +#define MACHINE_ID_LENGTH 32
> +
> +static void *__machine_id_hashable;
> +static size_t __machine_id_hashable_length;
> +
> +
> +void machine_id_set_hashable(void *hashable, size_t len)

const void *

> +{
> +	__machine_id_hashable = hashable;
> +	__machine_id_hashable_length = len;

You should make a copy rather than storing the pointer.

> +}
> +
> +static int machine_id_set_bootarg(void)
> +{
> +	struct digest *digest = NULL;
> +	unsigned char machine_id[SHA1_DIGEST_SIZE];
> +	char *hex_id;
> +	int ret = 0;
> +
> +	if (!__machine_id_hashable) {
> +		pr_warn("No hashable set, will not pass id to kernel\n");

I don't think this is worth a warning. With this series this will be
issued for example on all non i.MX6 boards enabled in imx_v7_defconfig.

> +		goto out;
> +	}
> +
> +	hex_id = "systemd.machine_id=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx";

This string should be treated const, better allocate hex_id.

> +
> +	digest = digest_alloc_by_algo(HASH_ALGO_SHA1);
> +	ret = digest_init(digest);
> +	if (ret)
> +		goto out;
> +
> +	ret = digest_update(digest, &__machine_id_hashable,
> +			    __machine_id_hashable_length);

You are hashing the address of the __machine_id_hashable pointer here.

> +	if (ret)
> +		goto out;
> +
> +	ret = digest_final(digest, machine_id);
> +	if (ret)
> +		goto out;
> +
> +	/* use the first 16 bytes of the sha1 hash as the machine-id */
> +	bin2hex(&hex_id[19], &machine_id[0], MACHINE_ID_LENGTH/2);
> +
> +	globalvar_add_simple("linux.bootargs.machine_id", &hex_id[0]);
> +
> +out:
> +	digest_free(digest);
> +	return ret;
> +
> +}
> +late_initcall(machine_id_set_bootarg);
> diff --git a/include/machine_id.h b/include/machine_id.h
> new file mode 100644
> index 0000000000..e4a9dacd4d
> --- /dev/null
> +++ b/include/machine_id.h
> @@ -0,0 +1,6 @@
> +#ifndef __MACHINE_ID_H__
> +#define __MACHINE_ID_H__
> +
> +void machine_id_set_hashable(void *hashable, size_t len);

You should add a static inline wrapper for CONFIG_MACHINE_ID disabled.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox