mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Marco Felsch <m.felsch@pengutronix.de>
Cc: "open list:BAREBOX" <barebox@lists.infradead.org>
Subject: Re: [PATCH v2 05/14] fip: rework fip_image_open()
Date: Wed, 12 Mar 2025 12:02:36 +0100	[thread overview]
Message-ID: <Z9FpzJGBShGoi0Ji@pengutronix.de> (raw)
In-Reply-To: <20250311134207.x4o2ccin3jx6pdpj@pengutronix.de>

On Tue, Mar 11, 2025 at 02:42:07PM +0100, Marco Felsch wrote:
> >  struct fip_state *fip_image_open(const char *filename, size_t offset)
> >  {
> > @@ -470,11 +466,13 @@ struct fip_state *fip_image_open(const char *filename, size_t offset)
> >  	int ret;
> >  	int fd;
> >  	struct fip_state *fip_state;
> > -	LIST_HEAD(entries);
> >  	size_t fip_headers_size, total = 0;
> > -	struct fip_image_desc *desc;
> >  	off_t pos;
> >  	int n_entries = 0;
> > +	struct fip_toc_entry toc_entries[16];
> 				 	 ^
> Why did you used 16?

Okay okay, I allocated the entries dynamically now to support an
arbitrary number of toc entries.

Sascha

-------------------------------8<------------------------------

>From 87bc7468e698e3404345ee601885ec186de7f4fa Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 11 Mar 2025 13:25:18 +0100
Subject: [PATCH] fip: rework fip_image_open()

fip_image_open() used to do all the parsing into a struct fip_state
itself. Instead, only load the FIP image into a buffer and call
fip_do_parse_buf() with this buffer. This has the advantage that we
have all parsing of the FIP image in a single place. Also this helps
with a followup patch which calculates a sha256 over a FIP image
which can easily done when we have the whole FIP image in a contiguous
buffer.

Link: https://lore.kernel.org/r/20250311-am625-secure-v2-5-3cbbfa092346@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 lib/fip.c | 90 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 53 insertions(+), 37 deletions(-)

diff --git a/lib/fip.c b/lib/fip.c
index 23e82098da..0d4ea54605 100644
--- a/lib/fip.c
+++ b/lib/fip.c
@@ -23,6 +23,8 @@
 #include <string.h>
 #include <libfile.h>
 #include <fs.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
 
 #include <fip.h>
 #include <fiptool.h>
@@ -446,23 +448,23 @@ int fip_update(struct fip_state *fip)
 	return 0;
 }
 
+struct toc_entry_list {
+	struct fip_toc_entry toc;
+	struct list_head list;
+};
+
 /*
- * fip_image_open - open a FIP image for readonly access
+ * fip_image_open - open a FIP image
  * @filename: The filename of the FIP image
  * @offset: The offset of the FIP image in the file
  *
- * This opens a FIP image for readonly access. This is an alternative
- * implementation for fip_parse() with these differences:
+ * This opens a FIP image. This is an alternative implementation for
+ * fip_parse() with these differences:
  * - suitable for reading FIP images from raw partitions. This function
  *   only reads the FIP image, even when the partition is bigger than the
  *   image
  * - Allows to specify an offset within the partition where the FIP image
  *   starts
- * - Do not memdup the images from the full FIP image
- *
- * This function is for easy readonly access to the images within the FIP
- * image. Do not call any of the above FIP manipulation functions other than
- * fip_free() on an image opened with this function.
  */
 struct fip_state *fip_image_open(const char *filename, size_t offset)
 {
@@ -470,11 +472,13 @@ struct fip_state *fip_image_open(const char *filename, size_t offset)
 	int ret;
 	int fd;
 	struct fip_state *fip_state;
-	LIST_HEAD(entries);
 	size_t fip_headers_size, total = 0;
-	struct fip_image_desc *desc;
 	off_t pos;
 	int n_entries = 0;
+	void *buf, *ptr;
+	struct fip_toc_entry *toc_entry;
+	struct toc_entry_list *toc_entry_list, *tmp;
+	LIST_HEAD(toc_entries);
 
 	fd = open(filename, O_RDONLY);
 	if (fd < 0)
@@ -506,11 +510,10 @@ struct fip_state *fip_image_open(const char *filename, size_t offset)
 
 	/* read all toc entries */
 	while (1) {
-		struct fip_image_desc *desc = xzalloc(sizeof(*desc));
-		struct fip_image *image = xzalloc(sizeof(*image));
-		struct fip_toc_entry *toc_entry = &image->toc_e;
+		uint64_t this_end;
 
-		desc->image = image;
+		toc_entry_list = xzalloc(sizeof(*toc_entry_list));
+		toc_entry = &toc_entry_list->toc;
 
 		ret = read_full(fd, toc_entry, sizeof(*toc_entry));
 		if (ret < 0)
@@ -520,54 +523,67 @@ struct fip_state *fip_image_open(const char *filename, size_t offset)
 			goto err;
 		}
 
-		list_add_tail(&desc->list, &fip_state->descs);
-
 		pr_debug("Read TOC entry %pU %llu %llu\n", &toc_entry->uuid,
 			 toc_entry->offset_address, toc_entry->size);
 
-		/* Found the ToC terminator, we are done. */
-		if (uuid_is_null(&toc_entry->uuid))
-			break;
-	}
-
-	/* determine buffer size */
-	fip_for_each_desc(fip_state, desc) {
-		uint64_t this_end = desc->image->toc_e.offset_address + desc->image->toc_e.size;
+		this_end = toc_entry->offset_address + toc_entry->size;
 
 		if (this_end > total)
 			total = this_end;
+
 		n_entries++;
-	}
 
-	fip_headers_size = n_entries * sizeof(struct fip_toc_entry) + sizeof(fip_toc_header_t);
+		list_add_tail(&toc_entry_list->list, &toc_entries);
 
-	total -= fip_headers_size;
+		/* Found the ToC terminator, we are done. */
+		if (uuid_is_null(&toc_entry->uuid))
+			break;
+	}
 
-	fip_state->buffer = malloc(total);
-	if (!fip_state->buffer) {
+	buf = malloc(total);
+	if (!buf) {
 		ret = -ENOMEM;
 		goto err;
 	}
 
-	ret = read_full(fd, fip_state->buffer, total);
+	ptr = buf;
+	fip_state->buffer = buf;
+
+	memcpy(ptr, &toc_header, sizeof(toc_header));
+	ptr += sizeof(toc_header);
+
+	list_for_each_entry_safe(toc_entry_list, tmp, &toc_entries, list) {
+		memcpy(ptr, &toc_entry_list->toc, sizeof(*toc_entry));
+		ptr += sizeof(*toc_entry);
+
+		list_del(&toc_entry_list->list);
+		free(toc_entry_list);
+	}
+
+	fip_headers_size = n_entries * sizeof(struct fip_toc_entry) + sizeof(fip_toc_header_t);
+
+	ret = read_full(fd, ptr, total - fip_headers_size);
+	ret = -EINVAL;
 	if (ret < 0)
 		goto err;
 
-	if (ret < total) {
+	if (ret < total - fip_headers_size) {
 		ret = -ENODATA;
 		goto err;
 	}
 
-	close(fd);
+	ret = fip_do_parse_buf(fip_state, buf, total, NULL);
+	if (ret)
+		goto err;
 
-	fip_for_each_desc(fip_state, desc) {
-		desc->image->buffer = fip_state->buffer +
-			desc->image->toc_e.offset_address - fip_headers_size;
-		desc->image->buf_no_free = true;
-	}
+	close(fd);
 
 	return fip_state;
+
 err:
+	list_for_each_entry_safe(toc_entry_list, tmp, &toc_entries, list)
+		free(toc_entry_list);
+
 	close(fd);
 	fip_free(fip_state);
 
-- 
2.39.5



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



  reply	other threads:[~2025-03-12 11:17 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-11 12:25 [PATCH v2 00/14] am625: support secure loading of full barebox Sascha Hauer
2025-03-11 12:25 ` [PATCH v2 01/14] firmware: always generate sha256sum Sascha Hauer
2025-03-11 13:13   ` Marco Felsch
2025-03-11 12:25 ` [PATCH v2 02/14] firmware: add function to verify next image Sascha Hauer
2025-03-11 13:19   ` Marco Felsch
2025-03-11 12:25 ` [PATCH v2 03/14] ARM: k3: r5: drop loading of separate binaries Sascha Hauer
2025-03-11 13:20   ` Marco Felsch
2025-03-11 12:25 ` [PATCH v2 04/14] ARM: k3: r5: add proper error handling Sascha Hauer
2025-03-11 13:21   ` Marco Felsch
2025-03-11 12:25 ` [PATCH v2 05/14] fip: rework fip_image_open() Sascha Hauer
2025-03-11 13:42   ` Marco Felsch
2025-03-12 11:02     ` Sascha Hauer [this message]
2025-03-12 11:45       ` Marco Felsch
2025-03-11 12:25 ` [PATCH v2 06/14] fip: fix wrong function call Sascha Hauer
2025-03-11 12:25 ` [PATCH v2 07/14] fip: add function to calculate a sha256 over FIP image Sascha Hauer
2025-03-11 13:43   ` Marco Felsch
2025-03-11 12:25 ` [PATCH v2 08/14] ARM: am625: support hash verification of full barebox Sascha Hauer
2025-03-11 13:44   ` Marco Felsch
2025-03-11 12:25 ` [PATCH v2 09/14] ARM: k3: add support for authenticating images against the ROM API Sascha Hauer
2025-03-11 12:25 ` [PATCH v2 10/14] ARM: k3: r5: delete fip image when it can't be opened Sascha Hauer
2025-03-11 12:25 ` [PATCH v2 11/14] ARM: k3: r5: Allow to authenticate next image by ROM API Sascha Hauer
2025-03-11 12:25 ` [PATCH v2 12/14] scripts/k3img: remove temporary files Sascha Hauer
2025-03-11 12:25 ` [PATCH v2 13/14] scripts: add k3sign Sascha Hauer
2025-03-11 12:25 ` [PATCH v2 14/14] ARM: k3: r5: select HAS_INSECURE_DEFAULTS when necessary Sascha Hauer
2025-03-11 13:46   ` Marco Felsch
2025-03-12 10:22 ` [PATCH v2 00/14] am625: support secure loading of full barebox Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z9FpzJGBShGoi0Ji@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=m.felsch@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox