On 25-03-11, Sascha Hauer wrote:
> 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.
>
> Signed-off-by: Sascha Hauer <s.ha...@pengutronix.de>
> ---
> lib/fip.c | 78
> +++++++++++++++++++++++++++++++++------------------------------
> 1 file changed, 41 insertions(+), 37 deletions(-)
>
> diff --git a/lib/fip.c b/lib/fip.c
> index 23e82098da..aab2795476 100644
> --- a/lib/fip.c
> +++ b/lib/fip.c
> @@ -23,6 +23,7 @@
> #include <string.h>
> #include <libfile.h>
> #include <fs.h>
> +#include <linux/kernel.h>
>
> #include <fip.h>
> #include <fiptool.h>
> @@ -447,22 +448,17 @@ int fip_update(struct fip_state *fip)
> }
>
> /*
> - * 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 +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?
> + void *buf, *ptr;
> + int i;
> + struct fip_toc_entry *toc_entry;
>
> fd = open(filename, O_RDONLY);
> if (fd < 0)
> @@ -506,11 +504,9 @@ 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 = &toc_entries[n_entries];
>
> ret = read_full(fd, toc_entry, sizeof(*toc_entry));
> if (ret < 0)
> @@ -520,53 +516,61 @@ 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);
> + if (n_entries >= ARRAY_SIZE(toc_entries)) {
pr_err("Error: Only 16 toc-entries are supported at the
moment\n");
> + ret = -ENOSPC;
> + goto err;
> + }
>
> - total -= fip_headers_size;
> + /* Found the ToC terminator, we are done. */
> + if (uuid_is_null(&toc_entry->uuid))
> + break;
If this check is happening after the n_entries check, we trigger a
false-positive in case of a 16-entries FIP.
Regards,
Marco
> + }
>
> - 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;
> +
> + memcpy(ptr, &toc_header, sizeof(toc_header));
> + ptr += sizeof(toc_header);
> +
> + for (i = 0; i < n_entries; i++) {
> + memcpy(ptr, &toc_entries[i], sizeof(*toc_entry));
> + ptr += sizeof(*toc_entry);
> + }
> +
> + fip_headers_size = n_entries * sizeof(struct fip_toc_entry) +
> sizeof(fip_toc_header_t);
> +
> + ret = read_full(fd, ptr, total - fip_headers_size);
> 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:
> close(fd);
> fip_free(fip_state);
>
> --
> 2.39.5
>
>
>
