[PATCH 1/4] string: add isempty helper

[PATCH 1/4] string: add isempty helper

[Marco Felsch]

This adds systemd's isempty() helper which can be very handy.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 include/string.h | 2 ++
 lib/string.c     | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/include/string.h b/include/string.h
index 43911b75762f..ffe7c42d3093 100644
--- a/include/string.h
+++ b/include/string.h
@@ -20,4 +20,6 @@ char *parse_assignment(char *str);
 
 int strverscmp(const char *a, const char *b);
 
+bool isempty(const char *s);
+
 #endif /* __STRING_H */
diff --git a/lib/string.c b/lib/string.c
index bf0f0455ab3f..20e6947347de 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -1000,3 +1000,9 @@ char *parse_assignment(char *str)
 
 	return value;
 }
+
+bool isempty(const char *s)
+{
+	return !s || s[0] == '\0';
+}
+EXPORT_SYMBOL(isempty);
-- 
2.39.2

[PATCH 2/4] common: env: make use of isempty()

[Marco Felsch]

Start making use of the new isempty() helper. No functional change.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/env.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/env.c b/common/env.c
index d673b061ab7a..3ffde8bbd1dc 100644
--- a/common/env.c
+++ b/common/env.c
@@ -336,10 +336,10 @@ const char *getenv_nonempty(const char *var)
 {
 	const char *val = getenv(var);
 
-	if (val && *val)
-		return val;
+	if (isempty(val))
+		return NULL;
 
-	return NULL;
+	return val;
 }
 EXPORT_SYMBOL(getenv_nonempty);
 
@@ -347,7 +347,7 @@ int getenv_ull(const char *var , unsigned long long *val)
 {
 	const char *valstr = getenv(var);
 
-	if (!valstr || !*valstr)
+	if (isempty(valstr))
 		return -EINVAL;
 
 	*val = simple_strtoull(valstr, NULL, 0);
@@ -360,7 +360,7 @@ int getenv_ul(const char *var , unsigned long *val)
 {
 	const char *valstr = getenv(var);
 
-	if (!valstr || !*valstr)
+	if (isempty(valstr))
 		return -EINVAL;
 
 	*val = simple_strtoul(valstr, NULL, 0);
@@ -373,7 +373,7 @@ int getenv_uint(const char *var , unsigned int *val)
 {
 	const char *valstr = getenv(var);
 
-	if (!valstr || !*valstr)
+	if (isempty(valstr))
 		return -EINVAL;
 
 	*val = simple_strtoul(valstr, NULL, 0);
-- 
2.39.2

[PATCH 3/4] common: hostname: validate the provided hostname

[Marco Felsch]

Port the systemd hostname_is_valid() function to perform hostname
validation and inform the user about a invalid hostname.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/misc.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/common/misc.c b/common/misc.c
index e266f0951ee9..dc498ad0b318 100644
--- a/common/misc.c
+++ b/common/misc.c
@@ -13,6 +13,7 @@
 #include <led.h>
 #include <of.h>
 #include <restart.h>
+#include <string.h>
 #include <linux/stringify.h>
 
 int errno;
@@ -146,6 +147,67 @@ static char *hostname;
 static char *serial_number;
 static char *of_machine_compatible;
 
+/* Note that HOST_NAME_MAX is 64 on Linux */
+#define BAREBOX_HOST_NAME_MAX	64
+
+static bool barebox_valid_ldh_char(char c)
+{
+	/* "LDH" -> "Letters, digits, hyphens", as per RFC 5890, Section 2.3.1 */
+	return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') ||
+	       (c >= '0' && c <= '9') || c == '-';
+}
+
+static bool barebox_hostname_is_valid(const char *s)
+{
+	unsigned int n_dots = 0;
+	const char *p;
+	bool dot, hyphen;
+
+	/*
+	 * Check if s looks like a valid hostname or FQDN. This does not do full
+	 * DNS validation, but only checks if the name is composed of allowed
+	 * characters and the length is not above the maximum allowed by Linux.
+	 * Doesn't accept empty hostnames, hostnames with leading dots, and
+	 * hostnames with multiple dots in a sequence. Doesn't allow hyphens at
+	 * the beginning or end of label.
+	 */
+	if (isempty(s))
+		return false;
+
+	for (p = s, dot = hyphen = true; *p; p++) {
+		if (*p == '.') {
+			if (dot || hyphen)
+				return false;
+
+			dot = true;
+			hyphen = false;
+			n_dots++;
+
+		} else if (*p == '-') {
+			if (dot)
+				return false;
+
+			dot = false;
+			hyphen = true;
+
+		} else {
+			if (!barebox_valid_ldh_char(*p))
+				return false;
+
+			dot = false;
+			hyphen = false;
+		}
+	}
+
+	if (dot || hyphen)
+		return false;
+
+	if (p - s > BAREBOX_HOST_NAME_MAX)
+		return false;
+
+	return true;
+}
+
 /*
  * The hostname is supposed to be the shortname of a board. It should
  * contain only lowercase letters, numbers, '-', '_'. No whitespaces
@@ -156,6 +218,10 @@ void barebox_set_hostname(const char *__hostname)
 	globalvar_add_simple_string("hostname", &hostname);
 
 	free(hostname);
+
+	if (!barebox_hostname_is_valid(__hostname))
+		pr_warn("Hostname is not valid, please fix it\n");
+
 	hostname = xstrdup(__hostname);
 }
 
-- 
2.39.2

[PATCH 4/4] bootm: add global.bootm.provide_hostname option

[Marco Felsch]

Add a new bootm option to automatically provide the $global.hostname via
the kernel commandline parameter systemd.hostname. The logic is based on
the provide_machine_id logic. To only provide valid hostnames the new
barebox_hostname_is_valid() is used therefore we need to make this
function public.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/bootm.c   | 25 +++++++++++++++++++++++++
 common/misc.c    |  2 +-
 include/bootm.h  |  5 +++++
 include/common.h |  1 +
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/common/bootm.c b/common/bootm.c
index 29ea13e28cb8..17d273831558 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -44,6 +44,7 @@ static struct image_handler *bootm_find_handler(enum filetype filetype,
 static int bootm_appendroot;
 static int bootm_earlycon;
 static int bootm_provide_machine_id;
+static int bootm_provide_hostname;
 static int bootm_verbosity;
 
 void bootm_data_init_defaults(struct bootm_data *data)
@@ -61,6 +62,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
 	data->verify = bootm_get_verify_mode();
 	data->appendroot = bootm_appendroot;
 	data->provide_machine_id = bootm_provide_machine_id;
+	data->provide_hostname = bootm_provide_hostname;
 	data->verbose = bootm_verbosity;
 }
 
@@ -797,6 +799,27 @@ int bootm_boot(struct bootm_data *bootm_data)
 		free(machine_id_bootarg);
 	}
 
+	if (bootm_data->provide_hostname) {
+		const char *hostname = getenv_nonempty("global.hostname");
+		char *hostname_bootarg;
+
+		if (!hostname) {
+			pr_err("Providing hostname is enabled but no hostname is set\n");
+			ret = -EINVAL;
+			goto err_out;
+		}
+
+		if (!barebox_hostname_is_valid(hostname)) {
+			pr_err("Provided hostname is not compatible to systemd hostname requirements\n");
+			ret = -EINVAL;
+			goto err_out;
+		}
+
+		hostname_bootarg = basprintf("systemd.hostname=%s", hostname);
+		globalvar_add_simple("linux.bootargs.hostname", hostname_bootarg);
+		free(hostname_bootarg);
+	}
+
 	pr_info("\nLoading %s '%s'", file_type_to_string(os_type),
 		data->os_file);
 	if (os_type == filetype_uimage &&
@@ -950,6 +973,7 @@ static int bootm_init(void)
 	globalvar_add_simple_bool("bootm.appendroot", &bootm_appendroot);
 	globalvar_add_simple_bool("bootm.earlycon", &bootm_earlycon);
 	globalvar_add_simple_bool("bootm.provide_machine_id", &bootm_provide_machine_id);
+	globalvar_add_simple_bool("bootm.provide_hostname", &bootm_provide_hostname);
 	if (IS_ENABLED(CONFIG_BOOTM_INITRD)) {
 		globalvar_add_simple("bootm.initrd", NULL);
 		globalvar_add_simple("bootm.initrd.loadaddr", NULL);
@@ -992,3 +1016,4 @@ BAREBOX_MAGICVAR(global.bootm.earlycon, "Add earlycon option to Kernel for early
 BAREBOX_MAGICVAR(global.bootm.appendroot, "Add root= option to Kernel to mount rootfs from the device the Kernel comes from (default, device can be overridden via global.bootm.root_dev)");
 BAREBOX_MAGICVAR(global.bootm.root_dev, "bootm default root device (overrides default device in global.bootm.appendroot)");
 BAREBOX_MAGICVAR(global.bootm.provide_machine_id, "If true, append systemd.machine_id=$global.machine_id to Kernel command line");
+BAREBOX_MAGICVAR(global.bootm.provide_hostname, "If true, append systemd.hostname=$global.hostname to Kernel command line");
diff --git a/common/misc.c b/common/misc.c
index dc498ad0b318..423af0005be0 100644
--- a/common/misc.c
+++ b/common/misc.c
@@ -157,7 +157,7 @@ static bool barebox_valid_ldh_char(char c)
 	       (c >= '0' && c <= '9') || c == '-';
 }
 
-static bool barebox_hostname_is_valid(const char *s)
+bool barebox_hostname_is_valid(const char *s)
 {
 	unsigned int n_dots = 0;
 	const char *p;
diff --git a/include/bootm.h b/include/bootm.h
index ee2b574521db..c69da85cdda1 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -34,6 +34,11 @@ struct bootm_data {
 	 * value of global.machine_id to Kernel.
 	 */
 	bool provide_machine_id;
+	/*
+	 * provide_hostname - if true, try to add systemd.hostname= with value
+	 * of global.hostname to Kernel.
+	 */
+	bool provide_hostname;
 	unsigned long initrd_address;
 	unsigned long os_address;
 	unsigned long os_entry;
diff --git a/include/common.h b/include/common.h
index b7b4d9e35094..d7b5261bc921 100644
--- a/include/common.h
+++ b/include/common.h
@@ -127,6 +127,7 @@ void barebox_set_model(const char *);
 const char *barebox_get_hostname(void);
 void barebox_set_hostname(const char *);
 void barebox_set_hostname_no_overwrite(const char *);
+bool barebox_hostname_is_valid(const char *s);
 
 const char *barebox_get_serial_number(void);
 void barebox_set_serial_number(const char *);
-- 
2.39.2

Re: [PATCH 4/4] bootm: add global.bootm.provide_hostname option

[Ahmad Fatoum]

Hello Marco,

Cc += Leonard

On 13.03.24 20:45, Marco Felsch wrote:
> +		if (!barebox_hostname_is_valid(hostname)) {
> +			pr_err("Provided hostname is not compatible to systemd hostname requirements\n");
> +			ret = -EINVAL;
> +			goto err_out;
> +		}
> +
> +		hostname_bootarg = basprintf("systemd.hostname=%s", hostname);

Linux >= v6.0 supports a hostname= parameter and there's a discussion to decide what
the semantics should be and if it should differ to systemd.hostname=:
https://github.com/systemd/systemd/pull/25158

I am not fully sure, whether we should go with hostname or systemd.hostname= here.
@Leonard, what do you think?

Cheers,
Ahmad


> -static bool barebox_hostname_is_valid(const char *s)
> +bool barebox_hostname_is_valid(const char *s)
>  {
>  	unsigned int n_dots = 0;
>  	const char *p;
> diff --git a/include/bootm.h b/include/bootm.h
> index ee2b574521db..c69da85cdda1 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -34,6 +34,11 @@ struct bootm_data {
>  	 * value of global.machine_id to Kernel.
>  	 */
>  	bool provide_machine_id;
> +	/*
> +	 * provide_hostname - if true, try to add systemd.hostname= with value
> +	 * of global.hostname to Kernel.
> +	 */
> +	bool provide_hostname;
>  	unsigned long initrd_address;
>  	unsigned long os_address;
>  	unsigned long os_entry;
> diff --git a/include/common.h b/include/common.h
> index b7b4d9e35094..d7b5261bc921 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -127,6 +127,7 @@ void barebox_set_model(const char *);
>  const char *barebox_get_hostname(void);
>  void barebox_set_hostname(const char *);
>  void barebox_set_hostname_no_overwrite(const char *);
> +bool barebox_hostname_is_valid(const char *s);
>  
>  const char *barebox_get_serial_number(void);
>  void barebox_set_serial_number(const char *);

-- 
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 |

Re: [PATCH 4/4] bootm: add global.bootm.provide_hostname option

[Marco Felsch]

Hi Ahmad,

On 24-03-14, Ahmad Fatoum wrote:
> Hello Marco,
> 
> Cc += Leonard
> 
> On 13.03.24 20:45, Marco Felsch wrote:
> > +		if (!barebox_hostname_is_valid(hostname)) {
> > +			pr_err("Provided hostname is not compatible to systemd hostname requirements\n");
> > +			ret = -EINVAL;
> > +			goto err_out;
> > +		}
> > +
> > +		hostname_bootarg = basprintf("systemd.hostname=%s", hostname);
> 
> Linux >= v6.0 supports a hostname= parameter and there's a discussion to decide what
> the semantics should be and if it should differ to systemd.hostname=:
> https://github.com/systemd/systemd/pull/25158

Thanks for the pointer, I think this discussion is still ongoing and
after skipping through the longly thread my impression is that the
semantics are the same.

> I am not fully sure, whether we should go with hostname or systemd.hostname= here.

As you have said, the hostname= param was added with v6.0 so by this
change distros using older kernels could benefit. Also I think that
hostname and systemd.hostname should be the same, to cite the GH
discussion:
8<-------------------------------------------------------------------
...

If there was already hostname= with those semantics on the kernel side
in place, I'm pretty sure we would've just reused it instead of defining
systemd.hostname=.

...
8<-------------------------------------------------------------------

Therefore I would keep the systemd.hostname as it was done by this
patch or we can supply both systemd.hostname= and hostname= but setting
it to the same value.

Regards,
  Marco

> @Leonard, what do you think?
> 
> Cheers,
> Ahmad
> 
> 
> > -static bool barebox_hostname_is_valid(const char *s)
> > +bool barebox_hostname_is_valid(const char *s)
> >  {
> >  	unsigned int n_dots = 0;
> >  	const char *p;
> > diff --git a/include/bootm.h b/include/bootm.h
> > index ee2b574521db..c69da85cdda1 100644
> > --- a/include/bootm.h
> > +++ b/include/bootm.h
> > @@ -34,6 +34,11 @@ struct bootm_data {
> >  	 * value of global.machine_id to Kernel.
> >  	 */
> >  	bool provide_machine_id;
> > +	/*
> > +	 * provide_hostname - if true, try to add systemd.hostname= with value
> > +	 * of global.hostname to Kernel.
> > +	 */
> > +	bool provide_hostname;
> >  	unsigned long initrd_address;
> >  	unsigned long os_address;
> >  	unsigned long os_entry;
> > diff --git a/include/common.h b/include/common.h
> > index b7b4d9e35094..d7b5261bc921 100644
> > --- a/include/common.h
> > +++ b/include/common.h
> > @@ -127,6 +127,7 @@ void barebox_set_model(const char *);
> >  const char *barebox_get_hostname(void);
> >  void barebox_set_hostname(const char *);
> >  void barebox_set_hostname_no_overwrite(const char *);
> > +bool barebox_hostname_is_valid(const char *s);
> >  
> >  const char *barebox_get_serial_number(void);
> >  void barebox_set_serial_number(const char *);
> 
> -- 
> 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 |
> 
> 

Re: [PATCH 1/4] string: add isempty helper

[Sascha Hauer]

On Wed, 13 Mar 2024 20:45:44 +0100, Marco Felsch wrote:
> This adds systemd's isempty() helper which can be very handy.
> 
> 

Applied, thanks!

[1/4] string: add isempty helper
      https://git.pengutronix.de/cgit/barebox/commit/?id=815db76bd8c8 (link may not be stable)
[2/4] common: env: make use of isempty()
      https://git.pengutronix.de/cgit/barebox/commit/?id=f681d1c0ebd0 (link may not be stable)
[3/4] common: hostname: validate the provided hostname
      https://git.pengutronix.de/cgit/barebox/commit/?id=e0d5d8bf9a81 (link may not be stable)
[4/4] bootm: add global.bootm.provide_hostname option
      https://git.pengutronix.de/cgit/barebox/commit/?id=c1bb8f33009e (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>