[RFC PATCH 0/3] bootconfig: Use hexadecimal ASCII string for size and checksum

[RFC PATCH 0/3] bootconfig: Use hexadecimal ASCII string for size and checksum

[Masami Hiramatsu]

Hello,

Here is a seires of patches to change the bootconfig footer format
to use 8-bytes hexadecimal ASCII string for size and checksum instead
of u32.

In the previous thread for alignment series[1], Steve pointed that
the current footer format makes it hard to apply the bootconfig for
cross-build initrd if the target endianness is different from the
host machine.

So, this changes the size and checksum format to 8-bytes hexadecimal
ASCII string as same as cpio. Since that is a string, we don't need
to care of the difference of the endianness anymore.

Thank you,

[1] https://lore.kernel.org/lkml/20201118112249.30d20147@gandalf.local.home/

---

Masami Hiramatsu (3):
      bootconfig: Use hexadecimal ASCII string for size and checksum
      tools/bootconfig: Use hexadecimal ASCII string for size and checksum
      docs: bootconfig: Use hexadecimal ASCII string for size and checksum


 Documentation/admin-guide/bootconfig.rst |    6 +++-
 init/main.c                              |   20 ++++++++++----
 tools/bootconfig/main.c                  |   43 +++++++++++++++++++-----------
 tools/bootconfig/test-bootconfig.sh      |    2 +
 4 files changed, 48 insertions(+), 23 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[RFC PATCH 1/3] bootconfig: Use hexadecimal ASCII string for size and checksum

[Masami Hiramatsu]

To make the bootconfig format more platform independent, use
8-bytes hexadecimal ASCII string for size and checksum field
in the footer. This will allow us to apply bootconfig to the
cross build initrd without caring the endianness.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 init/main.c |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/init/main.c b/init/main.c
index 20baced721ad..b82f23cff709 100644
--- a/init/main.c
+++ b/init/main.c
@@ -267,8 +267,8 @@ early_param("loglevel", loglevel);
 static void * __init get_boot_config_from_initrd(u32 *_size, u32 *_csum)
 {
 	u32 size, csum;
+	char buf[9];
 	char *data;
-	u32 *hdr;
 	int i;
 
 	if (!initrd_end)
@@ -287,11 +287,21 @@ static void * __init get_boot_config_from_initrd(u32 *_size, u32 *_csum)
 	return NULL;
 
 found:
-	hdr = (u32 *)(data - 8);
-	size = hdr[0];
-	csum = hdr[1];
+	buf[8] = '\0';
+	data -= 8;
+	memcpy(buf, data, 8);
+	if (kstrtou32(buf, 16, &csum) != 0) {
+		pr_err("bootconfig checksum field is invalid\n");
+		return NULL;
+	}
+	data -= 8;
+	memcpy(buf, data, 8);
+	if (kstrtou32(buf, 16, &size) != 0) {
+		pr_err("bootconfig size field is invalid\n");
+		return NULL;
+	}
+	data -= size;
 
-	data = ((void *)hdr) - size;
 	if ((unsigned long)data < initrd_start) {
 		pr_err("bootconfig size %d is greater than initrd size %ld\n",
 			size, initrd_end - initrd_start);

[RFC PATCH 2/3] tools/bootconfig: Use hexadecimal ASCII string for size and checksum

[Masami Hiramatsu]

To make the bootconfig format more platform independent, use
8-bytes hexadecimal ASCII string for size and checksum field
in the footer. This will allow us to apply bootconfig to the
cross build initrd without caring the endianness.

This commit updates bootconfig and its test so that it can handle
new format.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/bootconfig/main.c             |   43 ++++++++++++++++++++++-------------
 tools/bootconfig/test-bootconfig.sh |    2 +-
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/tools/bootconfig/main.c b/tools/bootconfig/main.c
index 4a445b6304bb..16eb5d4b9947 100644
--- a/tools/bootconfig/main.c
+++ b/tools/bootconfig/main.c
@@ -155,11 +155,12 @@ static int pr_errno(const char *msg, int err)
 
 static int load_xbc_from_initrd(int fd, char **buf)
 {
-	struct stat stat;
-	int ret;
-	u32 size = 0, csum = 0, rcsum;
 	char magic[BOOTCONFIG_MAGIC_LEN];
+	u32 size = 0, csum = 0, rcsum;
+	struct stat stat;
 	const char *msg;
+	char sbuf[9], *p;
+	int ret;
 
 	ret = fstat(fd, &stat);
 	if (ret < 0)
@@ -178,22 +179,33 @@ static int load_xbc_from_initrd(int fd, char **buf)
 	if (memcmp(magic, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN) != 0)
 		return 0;
 
-	if (lseek(fd, -(8 + BOOTCONFIG_MAGIC_LEN), SEEK_END) < 0)
+	if (lseek(fd, -(16 + BOOTCONFIG_MAGIC_LEN), SEEK_END) < 0)
 		return pr_errno("Failed to lseek for size", -errno);
 
-	if (read(fd, &size, sizeof(u32)) < 0)
+	sbuf[8] = '\0';
+	if (read(fd, sbuf, 8) < 0)
 		return pr_errno("Failed to read size", -errno);
+	size = strtoul(sbuf, &p, 16);
+	if (p != sbuf + 8) {
+		pr_err("Found invalid size field\n");
+		return -EINVAL;
+	}
 
-	if (read(fd, &csum, sizeof(u32)) < 0)
+	if (read(fd, sbuf, 8) < 0)
 		return pr_errno("Failed to read checksum", -errno);
+	csum = strtoul(sbuf, &p, 16);
+	if (p != sbuf + 8) {
+		pr_err("Found invalid checksum field\n");
+		return -EINVAL;
+	}
 
 	/* Wrong size error  */
-	if (stat.st_size < size + 8 + BOOTCONFIG_MAGIC_LEN) {
+	if (stat.st_size < size + 16 + BOOTCONFIG_MAGIC_LEN) {
 		pr_err("bootconfig size is too big\n");
 		return -E2BIG;
 	}
 
-	if (lseek(fd, stat.st_size - (size + 8 + BOOTCONFIG_MAGIC_LEN),
+	if (lseek(fd, stat.st_size - (size + 16 + BOOTCONFIG_MAGIC_LEN),
 		  SEEK_SET) < 0)
 		return pr_errno("Failed to lseek", -errno);
 
@@ -324,7 +336,7 @@ static int delete_xbc(const char *path)
 		ret = fstat(fd, &stat);
 		if (!ret)
 			ret = ftruncate(fd, stat.st_size
-					- size - 8 - BOOTCONFIG_MAGIC_LEN);
+					- size - 16 - BOOTCONFIG_MAGIC_LEN);
 		if (ret)
 			ret = -errno;
 	} /* Ignore if there is no boot config in initrd */
@@ -354,8 +366,7 @@ static int apply_xbc(const char *path, const char *xbc_path)
 	csum = checksum((unsigned char *)buf, size);
 
 	/* Backup the bootconfig data */
-	data = calloc(size + BOOTCONFIG_ALIGN +
-		      sizeof(u32) + sizeof(u32) + BOOTCONFIG_MAGIC_LEN, 1);
+	data = calloc(size + BOOTCONFIG_ALIGN + 16 + BOOTCONFIG_MAGIC_LEN, 1);
 	if (!data)
 		return -ENOMEM;
 	memcpy(data, buf, size);
@@ -401,17 +412,17 @@ static int apply_xbc(const char *path, const char *xbc_path)
 	}
 
 	/* To align up the total size to BOOTCONFIG_ALIGN, get padding size */
-	total_size = stat.st_size + size + sizeof(u32) * 2 + BOOTCONFIG_MAGIC_LEN;
+	total_size = stat.st_size + size + 16 + BOOTCONFIG_MAGIC_LEN;
 	pad = ((total_size + BOOTCONFIG_ALIGN - 1) & (~BOOTCONFIG_ALIGN_MASK)) - total_size;
 	size += pad;
 
 	/* Add a footer */
 	p = data + size;
-	*(u32 *)p = size;
-	p += sizeof(u32);
+	sprintf(p, "%08x", size);
+	p += 8;
 
-	*(u32 *)p = csum;
-	p += sizeof(u32);
+	sprintf(p, "%08x", csum);
+	p += 8;
 
 	memcpy(p, BOOTCONFIG_MAGIC, BOOTCONFIG_MAGIC_LEN);
 	p += BOOTCONFIG_MAGIC_LEN;
diff --git a/tools/bootconfig/test-bootconfig.sh b/tools/bootconfig/test-bootconfig.sh
index baed891d0ba4..a40926b8927c 100755
--- a/tools/bootconfig/test-bootconfig.sh
+++ b/tools/bootconfig/test-bootconfig.sh
@@ -60,7 +60,7 @@ echo "Show command test"
 xpass $BOOTCONF $INITRD
 
 echo "File size check"
-total_size=$(expr $bconf_size + $initrd_size + 9 + 12 + $ALIGN - 1 )
+total_size=$(expr $bconf_size + $initrd_size + 17 + 12 + $ALIGN - 1 )
 total_size=$(expr $total_size / $ALIGN)
 total_size=$(expr $total_size \* $ALIGN)
 xpass test $new_size -eq $total_size

[RFC PATCH 3/3] docs: bootconfig: Use hexadecimal ASCII string for size and checksum

[Masami Hiramatsu]

To make the bootconfig format more platform independent, use
8-bytes hexadecimal ASCII string for size and checksum field
in the footer. This will allow us to apply bootconfig to the
cross build initrd without caring the endianness.

This commit updates the document to define the format.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Documentation/admin-guide/bootconfig.rst |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index 363599683784..1c6d6919d9e6 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -140,7 +140,11 @@ Since the boot configuration file is loaded with initrd, it will be added
 to the end of the initrd (initramfs) image file with padding, size,
 checksum and 12-byte magic word as below.
 
-[initrd][bootconfig][padding][size(u32)][checksum(u32)][#BOOTCONFIG\n]
+[initrd][bootconfig][padding][size][checksum][#BOOTCONFIG\n]
+
+The size and checksum fields are 8 bytes hexadecimal ASCII numbers fully
+padded with '0' on the left to the full width of the field, for example,
+the integer 1234 is represented by the ASCII string "000004d2".
 
 When the boot configuration is added to the initrd image, the total
 file size is aligned to 4 bytes. To fill the gap, null characters

Re: [RFC PATCH 0/3] bootconfig: Use hexadecimal ASCII string for size and checksum

[Linus Torvalds]

On Thu, Nov 19, 2020 at 6:31 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> Here is a seires of patches to change the bootconfig footer format
> to use 8-bytes hexadecimal ASCII string for size and checksum instead
> of u32.

Ugh.,

Just make it little-endian only.

The _worst_ thing to do is to make it some kind of "native-endian",
because then you have to deal with cross building issues etc.

But using a __le32 type and just doing "le32_to_cpu()" is trivial and
optimal - not just because everybody relevant is LE anyway, but simply
because even if you _aren't_ LE, an unconditional byte swap is better
than a conditional native access.

           Linus

Re: [RFC PATCH 0/3] bootconfig: Use hexadecimal ASCII string for size and checksum

[Steven Rostedt]

On Thu, 19 Nov 2020 09:36:47 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Nov 19, 2020 at 6:31 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> >
> > Here is a seires of patches to change the bootconfig footer format
> > to use 8-bytes hexadecimal ASCII string for size and checksum instead
> > of u32.  
> 
> Ugh.,
> 
> Just make it little-endian only.
> 
> The _worst_ thing to do is to make it some kind of "native-endian",
> because then you have to deal with cross building issues etc.
> 
> But using a __le32 type and just doing "le32_to_cpu()" is trivial and
> optimal - not just because everybody relevant is LE anyway, but simply
> because even if you _aren't_ LE, an unconditional byte swap is better
> than a conditional native access.

And since this isn't used in any fast paths, the byte swapping in the
kernel should be a non-issue.

-- Steve

Re: [RFC PATCH 0/3] bootconfig: Use hexadecimal ASCII string for size and checksum

[Masami Hiramatsu]

On Thu, 19 Nov 2020 12:42:34 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 19 Nov 2020 09:36:47 -0800
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Thu, Nov 19, 2020 at 6:31 AM Masami Hiramatsu <mhiramat@kernel.org> wrote:
> > >
> > > Here is a seires of patches to change the bootconfig footer format
> > > to use 8-bytes hexadecimal ASCII string for size and checksum instead
> > > of u32.  
> > 
> > Ugh.,
> > 
> > Just make it little-endian only.
> > 
> > The _worst_ thing to do is to make it some kind of "native-endian",
> > because then you have to deal with cross building issues etc.
> > 
> > But using a __le32 type and just doing "le32_to_cpu()" is trivial and
> > optimal - not just because everybody relevant is LE anyway, but simply
> > because even if you _aren't_ LE, an unconditional byte swap is better
> > than a conditional native access.
> 
> And since this isn't used in any fast paths, the byte swapping in the
> kernel should be a non-issue.

Thanks for the comment!
OK, let me renew the series to use le32.


-- 
Masami Hiramatsu <mhiramat@kernel.org>