[PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[Rasmus Villemoes]

Background:

I have several customers that will be using a certain remote signing
service for signing their images, in order that the private keys are
never exposed outside that company's secure servers. This is done via
a pkcs#11 interface that talks to the remote signing server, and all
of that works quite well.

However, the way this particular signing service works is that one
must upfront create a "signing session", where one indicates which
keys one will use and, importantly, how many times each key will (may)
be used. Then, depending on the keys requested and the customer's
configuration, one or more humans must authorize that signing session
So for example, if official release keys are to be used, maybe two
different people from upper management must authorize, while if
development keys are requested, the developer himself can authorize
the session.

Once authorized, the requester receives a token that must then be used
for signing via one of the keys associated to that session.

I have that integrated in Yocto in a way that when a CI starts a BSP
build, it automatically works out which keys will be needed (e.g. one
for signing U-Boot, another for signing a kernel FIT image) based on
bitbake metadata, requests an appropriate signing session, and the
appropriate people are then notified and can then look at the details
of that CI pipeline and confirm that it is legitimate.

The problem:

The way mkimage does FIT image signing means that the remote server
can be asked to perform a signature an unbounded number of times, or
at least a number of times that cannot be determined upfront. This
means that currently, I need to artificially say that a kernel key
will be used, say, 10 times, even when only a single FIT image with
just one configuration node is created.

Part of the security model is that once the number of signings using a
given key has been depleted, the authorization token becomes useless
even if somehow leaked from the CI - and _if_ it is leaked/compromised
and abused before the CI has gotten around to do its signings, the
build will then fail with a clear indication of the
compromise. Clearly, having to specify a "high enough" expected use
count is counter to that part of the security model, because it will
inevitably leave some allowed uses behind.

While not perfect, we can give a reasonable estimate of an upper bound
on the necessary extra size by simply counting the number of hash and
signature nodes in the FIT image.

As indicated in the comments, one could probably make it even more
precise, and if there would ever be signatures larger than 512 bytes,
probably one would have to do that. But this works well enough in
practice for now, and is in fact an improvement in the normal case:
Currently, starting with size_inc of 0 is guaranteed to fail, so we
always enter the loop at least twice, even when not doing any signing
but merely filling hash values.

Just in case I've missed anything, keep the loop incrementing 1024
bytes at a time, and also, in case the estimate turns out to be over
64K, ensure that we do at least one attempt by changing to a do-while
loop.

With a little debug printf, creating a FIT image with three
configuration nodes previously resulted in

  Trying size_inc=0
  Trying size_inc=1024
  Trying size_inc=2048
  Trying size_inc=3072
  Succeeded at size_inc=3072

and dumping info from the signing session (where I've artifically
asked for 10 uses of the kernel key) shows

      "keyid": "kernel-dev-20250218",
      "usagecount": 9,
      "maxusagecount": 10

corresponding to 1+2+3+3 signatures requested (so while the loop count
is roughly linear in the number of config nodes, the number of
signings is quadratic).

With this, I instead get

  Trying size_inc=3456
  Succeeded at size_inc=3456

and the expected

      "keyid": "kernel-dev-20250218",
      "usagecount": 3,
      "maxusagecount": 10

thus allowing me to set maxusagecount correctly.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 tools/fit_image.c | 80 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 70 insertions(+), 10 deletions(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index caed8d5f901..65c83e0b950 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -24,6 +24,65 @@
 
 static struct legacy_img_hdr header;
 
+static int fit_estimate_hash_sig_size(struct image_tool_params *params, const char *fname)
+{
+	bool signing = IMAGE_ENABLE_SIGN && (params->keydir || params->keyfile);
+	struct stat sbuf;
+	void *fdt;
+	int fd;
+	int estimate = 0;
+	int depth, noffset;
+	const char *name;
+
+	fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, true);
+	if (fd < 0)
+		return -EIO;
+
+	/*
+	 * Walk the FIT image, looking for nodes named hash* and
+	 * signature*. Since the interesting nodes are subnodes of an
+	 * image or configuration node, we are only interested in
+	 * those at depth exactly 3.
+	 *
+	 * The estimate for a hash node is based on a sha512 digest
+	 * being 64 bytes, with another 64 bytes added to account for
+	 * fdt structure overhead (the tags and the name of the
+	 * "value" property).
+	 *
+	 * The estimate for a signature node is based on an rsa4096
+	 * signature being 512 bytes, with another 512 bytes to
+	 * account for fdt overhead and the various other properties
+	 * (hashed-nodes etc.) that will also be filled in.
+	 *
+	 * One could try to be more precise in the estimates by
+	 * looking at the "algo" property and, in the case of
+	 * configuration signatures, the sign-images property. Also,
+	 * when signing an already created FIT image, the hash nodes
+	 * already have properly sized value properties, so one could
+	 * also take pre-existence of "value" properties in hash nodes
+	 * into account. But this rather simple approach should work
+	 * well enough in practice.
+	 */
+	for (depth = 0, noffset = fdt_next_node(fdt, 0, &depth);
+	     noffset >= 0 && depth > 0;
+	     noffset = fdt_next_node(fdt, noffset, &depth)) {
+		if (depth != 3)
+			continue;
+
+		name = fdt_get_name(fdt, noffset, NULL);
+		if (!strncmp(name, FIT_HASH_NODENAME, strlen(FIT_HASH_NODENAME)))
+			estimate += 128;
+
+		if (signing && !strncmp(name, FIT_SIG_NODENAME, strlen(FIT_SIG_NODENAME)))
+			estimate += 1024;
+	}
+
+	munmap(fdt, sbuf.st_size);
+	close(fd);
+
+	return estimate;
+}
+
 static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
 			     const char *tmpfile)
 {
@@ -806,16 +865,16 @@ static int fit_handle_file(struct image_tool_params *params)
 	rename(tmpfile, bakfile);
 
 	/*
-	 * Set hashes for images in the blob. Unfortunately we may need more
-	 * space in either FDT, so keep trying until we succeed.
-	 *
-	 * Note: this is pretty inefficient for signing, since we must
-	 * calculate the signature every time. It would be better to calculate
-	 * all the data and then store it in a separate step. However, this
-	 * would be considerably more complex to implement. Generally a few
-	 * steps of this loop is enough to sign with several keys.
+	 * Set hashes for images in the blob and compute
+	 * signatures. We do an attempt at estimating the expected
+	 * extra size, but just in case that is not sufficient, keep
+	 * trying adding 1K, with a reasonable upper bound of 64K
+	 * total, until we succeed.
 	 */
-	for (size_inc = 0; size_inc < 64 * 1024; size_inc += 1024) {
+	size_inc = fit_estimate_hash_sig_size(params, bakfile);
+	if (size_inc < 0)
+		goto err_system;
+	do {
 		if (copyfile(bakfile, tmpfile) < 0) {
 			printf("Can't copy %s to %s\n", bakfile, tmpfile);
 			ret = -EIO;
@@ -824,7 +883,8 @@ static int fit_handle_file(struct image_tool_params *params)
 		ret = fit_add_file_data(params, size_inc, tmpfile);
 		if (!ret || ret != -ENOSPC)
 			break;
-	}
+		size_inc += 1024;
+	} while (size_inc < 64 * 1024);
 
 	if (ret) {
 		fprintf(stderr, "%s Can't add hashes to FIT blob: %d\n",
-- 
2.49.0

Re: [PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[Simon Glass]

Hi Rasmus,

On Fri, 16 May 2025 at 14:54, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> Background:
>
> I have several customers that will be using a certain remote signing
> service for signing their images, in order that the private keys are
> never exposed outside that company's secure servers. This is done via
> a pkcs#11 interface that talks to the remote signing server, and all
> of that works quite well.
>
> However, the way this particular signing service works is that one
> must upfront create a "signing session", where one indicates which
> keys one will use and, importantly, how many times each key will (may)
> be used. Then, depending on the keys requested and the customer's
> configuration, one or more humans must authorize that signing session
> So for example, if official release keys are to be used, maybe two
> different people from upper management must authorize, while if
> development keys are requested, the developer himself can authorize
> the session.
>
> Once authorized, the requester receives a token that must then be used
> for signing via one of the keys associated to that session.
>
> I have that integrated in Yocto in a way that when a CI starts a BSP
> build, it automatically works out which keys will be needed (e.g. one
> for signing U-Boot, another for signing a kernel FIT image) based on
> bitbake metadata, requests an appropriate signing session, and the
> appropriate people are then notified and can then look at the details
> of that CI pipeline and confirm that it is legitimate.
>
> The problem:
>
> The way mkimage does FIT image signing means that the remote server
> can be asked to perform a signature an unbounded number of times, or
> at least a number of times that cannot be determined upfront. This
> means that currently, I need to artificially say that a kernel key
> will be used, say, 10 times, even when only a single FIT image with
> just one configuration node is created.
>
> Part of the security model is that once the number of signings using a
> given key has been depleted, the authorization token becomes useless
> even if somehow leaked from the CI - and _if_ it is leaked/compromised
> and abused before the CI has gotten around to do its signings, the
> build will then fail with a clear indication of the
> compromise. Clearly, having to specify a "high enough" expected use
> count is counter to that part of the security model, because it will
> inevitably leave some allowed uses behind.
>
> While not perfect, we can give a reasonable estimate of an upper bound
> on the necessary extra size by simply counting the number of hash and
> signature nodes in the FIT image.
>
> As indicated in the comments, one could probably make it even more
> precise, and if there would ever be signatures larger than 512 bytes,
> probably one would have to do that. But this works well enough in
> practice for now, and is in fact an improvement in the normal case:
> Currently, starting with size_inc of 0 is guaranteed to fail, so we
> always enter the loop at least twice, even when not doing any signing
> but merely filling hash values.
>
> Just in case I've missed anything, keep the loop incrementing 1024
> bytes at a time, and also, in case the estimate turns out to be over
> 64K, ensure that we do at least one attempt by changing to a do-while
> loop.
>
> With a little debug printf, creating a FIT image with three
> configuration nodes previously resulted in
>
>   Trying size_inc=0
>   Trying size_inc=1024
>   Trying size_inc=2048
>   Trying size_inc=3072
>   Succeeded at size_inc=3072
>
> and dumping info from the signing session (where I've artifically
> asked for 10 uses of the kernel key) shows
>
>       "keyid": "kernel-dev-20250218",
>       "usagecount": 9,
>       "maxusagecount": 10
>
> corresponding to 1+2+3+3 signatures requested (so while the loop count
> is roughly linear in the number of config nodes, the number of
> signings is quadratic).
>
> With this, I instead get
>
>   Trying size_inc=3456
>   Succeeded at size_inc=3456
>
> and the expected
>
>       "keyid": "kernel-dev-20250218",
>       "usagecount": 3,
>       "maxusagecount": 10
>
> thus allowing me to set maxusagecount correctly.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> ---
>  tools/fit_image.c | 80 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/tools/fit_image.c b/tools/fit_image.c
> index caed8d5f901..65c83e0b950 100644
> --- a/tools/fit_image.c
> +++ b/tools/fit_image.c
> @@ -24,6 +24,65 @@
>
>  static struct legacy_img_hdr header;
>
> +static int fit_estimate_hash_sig_size(struct image_tool_params *params, const char *fname)
> +{
> +       bool signing = IMAGE_ENABLE_SIGN && (params->keydir || params->keyfile);
> +       struct stat sbuf;
> +       void *fdt;
> +       int fd;
> +       int estimate = 0;
> +       int depth, noffset;
> +       const char *name;
> +
> +       fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, true);
> +       if (fd < 0)
> +               return -EIO;
> +
> +       /*
> +        * Walk the FIT image, looking for nodes named hash* and
> +        * signature*. Since the interesting nodes are subnodes of an
> +        * image or configuration node, we are only interested in
> +        * those at depth exactly 3.
> +        *
> +        * The estimate for a hash node is based on a sha512 digest
> +        * being 64 bytes, with another 64 bytes added to account for
> +        * fdt structure overhead (the tags and the name of the
> +        * "value" property).
> +        *
> +        * The estimate for a signature node is based on an rsa4096
> +        * signature being 512 bytes, with another 512 bytes to
> +        * account for fdt overhead and the various other properties
> +        * (hashed-nodes etc.) that will also be filled in.
> +        *
> +        * One could try to be more precise in the estimates by
> +        * looking at the "algo" property and, in the case of
> +        * configuration signatures, the sign-images property. Also,
> +        * when signing an already created FIT image, the hash nodes
> +        * already have properly sized value properties, so one could
> +        * also take pre-existence of "value" properties in hash nodes
> +        * into account. But this rather simple approach should work
> +        * well enough in practice.
> +        */
> +       for (depth = 0, noffset = fdt_next_node(fdt, 0, &depth);
> +            noffset >= 0 && depth > 0;
> +            noffset = fdt_next_node(fdt, noffset, &depth)) {
> +               if (depth != 3)
> +                       continue;
> +
> +               name = fdt_get_name(fdt, noffset, NULL);
> +               if (!strncmp(name, FIT_HASH_NODENAME, strlen(FIT_HASH_NODENAME)))
> +                       estimate += 128;
> +
> +               if (signing && !strncmp(name, FIT_SIG_NODENAME, strlen(FIT_SIG_NODENAME)))
> +                       estimate += 1024;
> +       }
> +
> +       munmap(fdt, sbuf.st_size);
> +       close(fd);
> +
> +       return estimate;
> +}
> +
>  static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
>                              const char *tmpfile)
>  {
> @@ -806,16 +865,16 @@ static int fit_handle_file(struct image_tool_params *params)
>         rename(tmpfile, bakfile);
>
>         /*
> -        * Set hashes for images in the blob. Unfortunately we may need more
> -        * space in either FDT, so keep trying until we succeed.
> -        *
> -        * Note: this is pretty inefficient for signing, since we must
> -        * calculate the signature every time. It would be better to calculate
> -        * all the data and then store it in a separate step. However, this
> -        * would be considerably more complex to implement. Generally a few
> -        * steps of this loop is enough to sign with several keys.
> +        * Set hashes for images in the blob and compute
> +        * signatures. We do an attempt at estimating the expected
> +        * extra size, but just in case that is not sufficient, keep
> +        * trying adding 1K, with a reasonable upper bound of 64K
> +        * total, until we succeed.
>          */
> -       for (size_inc = 0; size_inc < 64 * 1024; size_inc += 1024) {
> +       size_inc = fit_estimate_hash_sig_size(params, bakfile);
> +       if (size_inc < 0)
> +               goto err_system;
> +       do {
>                 if (copyfile(bakfile, tmpfile) < 0) {
>                         printf("Can't copy %s to %s\n", bakfile, tmpfile);
>                         ret = -EIO;
> @@ -824,7 +883,8 @@ static int fit_handle_file(struct image_tool_params *params)
>                 ret = fit_add_file_data(params, size_inc, tmpfile);
>                 if (!ret || ret != -ENOSPC)
>                         break;
> -       }
> +               size_inc += 1024;
> +       } while (size_inc < 64 * 1024);
>
>         if (ret) {
>                 fprintf(stderr, "%s Can't add hashes to FIT blob: %d\n",
> --
> 2.49.0
>

I have no particular objection here if you really want to go this way,
but it seems to me that it would be better to have a way to determine
the size of signatures using a new (or existing?) API call. Then you
can make this deterministic and always correct.

Regards,
Simon

Re: [PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[Rasmus Villemoes]

On Fri, May 16 2025, Simon Glass <sjg@chromium.org> wrote:

> Hi Rasmus,
>
> On Fri, 16 May 2025 at 14:54, Rasmus Villemoes <ravi@prevas.dk> wrote:
>>
>>
>> While not perfect, we can give a reasonable estimate of an upper bound
>> on the necessary extra size by simply counting the number of hash and
>> signature nodes in the FIT image.
>>
>> As indicated in the comments, one could probably make it even more
>> precise, and if there would ever be signatures larger than 512 bytes,
>> probably one would have to do that. But this works well enough in
>> practice for now, and is in fact an improvement in the normal case:
>> Currently, starting with size_inc of 0 is guaranteed to fail, so we
>> always enter the loop at least twice, even when not doing any signing
>> but merely filling hash values.
>>
>
> I have no particular objection here if you really want to go this way,
> but it seems to me that it would be better to have a way to determine
> the size of signatures using a new (or existing?) API call. Then you
> can make this deterministic and always correct.

Well, I'm not saying I won't revisit this at some point in the future
and improve on the heuristics.

However, as indicated, it's not just a matter of knowing the exact size
of the signature data; one also has to account for the space used by the
"hashed-nodes" property, which of course depends on the number of
elements in the sign-images property, but also on the number of hash-*
nodes in those "pointed-to" images. So it ends up being quite a lot of
code to get it more exact.

So yes, for now I would like to go this way, because it's a fix that
works for my use case now and is easy to reason about, and should also
be an improvement outside my use case in that it avoids at least one
iteration, but possibly many.

Rasmus

Re: [PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[Simon Glass]

Hi Rasmus,

On Mon, 19 May 2025 at 11:28, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
> On Fri, May 16 2025, Simon Glass <sjg@chromium.org> wrote:
>
> > Hi Rasmus,
> >
> > On Fri, 16 May 2025 at 14:54, Rasmus Villemoes <ravi@prevas.dk> wrote:
> >>
> >>
> >> While not perfect, we can give a reasonable estimate of an upper bound
> >> on the necessary extra size by simply counting the number of hash and
> >> signature nodes in the FIT image.
> >>
> >> As indicated in the comments, one could probably make it even more
> >> precise, and if there would ever be signatures larger than 512 bytes,
> >> probably one would have to do that. But this works well enough in
> >> practice for now, and is in fact an improvement in the normal case:
> >> Currently, starting with size_inc of 0 is guaranteed to fail, so we
> >> always enter the loop at least twice, even when not doing any signing
> >> but merely filling hash values.
> >>
> >
> > I have no particular objection here if you really want to go this way,
> > but it seems to me that it would be better to have a way to determine
> > the size of signatures using a new (or existing?) API call. Then you
> > can make this deterministic and always correct.
>
> Well, I'm not saying I won't revisit this at some point in the future
> and improve on the heuristics.
>
> However, as indicated, it's not just a matter of knowing the exact size
> of the signature data; one also has to account for the space used by the
> "hashed-nodes" property, which of course depends on the number of
> elements in the sign-images property, but also on the number of hash-*
> nodes in those "pointed-to" images. So it ends up being quite a lot of
> code to get it more exact.
>
> So yes, for now I would like to go this way, because it's a fix that
> works for my use case now and is easy to reason about, and should also
> be an improvement outside my use case in that it avoids at least one
> iteration, but possibly many.

The 'right' way to do this might involve converting the APIs to use
ofnode, then enhancing ofnode to support tracking how much extra space
is needed (instead of actually updating the fdt), so you can then set
the size and do it for real.

Anyway, you seem keen on your current approach and it is better than
what we have.

Regards,
Simon

Re: [PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4611 bytes --]

On Fri, May 16, 2025 at 02:54:30PM +0200, Rasmus Villemoes wrote:

> Background:
> 
> I have several customers that will be using a certain remote signing
> service for signing their images, in order that the private keys are
> never exposed outside that company's secure servers. This is done via
> a pkcs#11 interface that talks to the remote signing server, and all
> of that works quite well.
> 
> However, the way this particular signing service works is that one
> must upfront create a "signing session", where one indicates which
> keys one will use and, importantly, how many times each key will (may)
> be used. Then, depending on the keys requested and the customer's
> configuration, one or more humans must authorize that signing session
> So for example, if official release keys are to be used, maybe two
> different people from upper management must authorize, while if
> development keys are requested, the developer himself can authorize
> the session.
> 
> Once authorized, the requester receives a token that must then be used
> for signing via one of the keys associated to that session.
> 
> I have that integrated in Yocto in a way that when a CI starts a BSP
> build, it automatically works out which keys will be needed (e.g. one
> for signing U-Boot, another for signing a kernel FIT image) based on
> bitbake metadata, requests an appropriate signing session, and the
> appropriate people are then notified and can then look at the details
> of that CI pipeline and confirm that it is legitimate.
> 
> The problem:
> 
> The way mkimage does FIT image signing means that the remote server
> can be asked to perform a signature an unbounded number of times, or
> at least a number of times that cannot be determined upfront. This
> means that currently, I need to artificially say that a kernel key
> will be used, say, 10 times, even when only a single FIT image with
> just one configuration node is created.
> 
> Part of the security model is that once the number of signings using a
> given key has been depleted, the authorization token becomes useless
> even if somehow leaked from the CI - and _if_ it is leaked/compromised
> and abused before the CI has gotten around to do its signings, the
> build will then fail with a clear indication of the
> compromise. Clearly, having to specify a "high enough" expected use
> count is counter to that part of the security model, because it will
> inevitably leave some allowed uses behind.
> 
> While not perfect, we can give a reasonable estimate of an upper bound
> on the necessary extra size by simply counting the number of hash and
> signature nodes in the FIT image.
> 
> As indicated in the comments, one could probably make it even more
> precise, and if there would ever be signatures larger than 512 bytes,
> probably one would have to do that. But this works well enough in
> practice for now, and is in fact an improvement in the normal case:
> Currently, starting with size_inc of 0 is guaranteed to fail, so we
> always enter the loop at least twice, even when not doing any signing
> but merely filling hash values.
> 
> Just in case I've missed anything, keep the loop incrementing 1024
> bytes at a time, and also, in case the estimate turns out to be over
> 64K, ensure that we do at least one attempt by changing to a do-while
> loop.
> 
> With a little debug printf, creating a FIT image with three
> configuration nodes previously resulted in
> 
>   Trying size_inc=0
>   Trying size_inc=1024
>   Trying size_inc=2048
>   Trying size_inc=3072
>   Succeeded at size_inc=3072
> 
> and dumping info from the signing session (where I've artifically
> asked for 10 uses of the kernel key) shows
> 
>       "keyid": "kernel-dev-20250218",
>       "usagecount": 9,
>       "maxusagecount": 10
> 
> corresponding to 1+2+3+3 signatures requested (so while the loop count
> is roughly linear in the number of config nodes, the number of
> signings is quadratic).
> 
> With this, I instead get
> 
>   Trying size_inc=3456
>   Succeeded at size_inc=3456
> 
> and the expected
> 
>       "keyid": "kernel-dev-20250218",
>       "usagecount": 3,
>       "maxusagecount": 10
> 
> thus allowing me to set maxusagecount correctly.
> 
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
> ---
>  tools/fit_image.c | 80 +++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 70 insertions(+), 10 deletions(-)

I think some tests need to be updated now:
https://source.denx.de/u-boot/u-boot/-/jobs/1156824

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[Rasmus Villemoes]

On Wed, Jun 04 2025, Tom Rini <trini@konsulko.com> wrote:

> On Fri, May 16, 2025 at 02:54:30PM +0200, Rasmus Villemoes wrote:
>
>> 
>> While not perfect, we can give a reasonable estimate of an upper bound
>> on the necessary extra size by simply counting the number of hash and
>> signature nodes in the FIT image.
>> 
>> As indicated in the comments, one could probably make it even more
>> precise, and if there would ever be signatures larger than 512 bytes,
>> probably one would have to do that. But this works well enough in
>> practice for now, and is in fact an improvement in the normal case:
>> Currently, starting with size_inc of 0 is guaranteed to fail, so we
>> always enter the loop at least twice, even when not doing any signing
>> but merely filling hash values.
>> 
>> Just in case I've missed anything, keep the loop incrementing 1024
>> bytes at a time, and also, in case the estimate turns out to be over
>> 64K, ensure that we do at least one attempt by changing to a do-while
>> loop.
>> 
>> ---
>>  tools/fit_image.c | 80 +++++++++++++++++++++++++++++++++++++++++------
>>  1 file changed, 70 insertions(+), 10 deletions(-)
>
> I think some tests need to be updated now:
> https://source.denx.de/u-boot/u-boot/-/jobs/1156824

Hm, yes.

I don't like blindly just updating such numbers so they match, but in
this case I think I can at least explain the delta: The fit image
described in 161_fit.dts has four hash nodes, so with my estimate we
first try with an increment of 4*128=512, while previously we'd first
try 0 and then 1024. Since 512 is enough, the fit image ends up 512
bytes smaller, and that is exactly the delta seen in fit:size and the
other numbers affected by that.

Should I fold in those changes and resend, or is a separate followup
patch better/ok?

Rasmus

Re: [PATCH] mkimage: do a rough estimate for the size needed for hashes/signatures

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

On Wed, Jun 04, 2025 at 10:19:41PM +0200, Rasmus Villemoes wrote:
> On Wed, Jun 04 2025, Tom Rini <trini@konsulko.com> wrote:
> 
> > On Fri, May 16, 2025 at 02:54:30PM +0200, Rasmus Villemoes wrote:
> >
> >> 
> >> While not perfect, we can give a reasonable estimate of an upper bound
> >> on the necessary extra size by simply counting the number of hash and
> >> signature nodes in the FIT image.
> >> 
> >> As indicated in the comments, one could probably make it even more
> >> precise, and if there would ever be signatures larger than 512 bytes,
> >> probably one would have to do that. But this works well enough in
> >> practice for now, and is in fact an improvement in the normal case:
> >> Currently, starting with size_inc of 0 is guaranteed to fail, so we
> >> always enter the loop at least twice, even when not doing any signing
> >> but merely filling hash values.
> >> 
> >> Just in case I've missed anything, keep the loop incrementing 1024
> >> bytes at a time, and also, in case the estimate turns out to be over
> >> 64K, ensure that we do at least one attempt by changing to a do-while
> >> loop.
> >> 
> >> ---
> >>  tools/fit_image.c | 80 +++++++++++++++++++++++++++++++++++++++++------
> >>  1 file changed, 70 insertions(+), 10 deletions(-)
> >
> > I think some tests need to be updated now:
> > https://source.denx.de/u-boot/u-boot/-/jobs/1156824
> 
> Hm, yes.
> 
> I don't like blindly just updating such numbers so they match, but in
> this case I think I can at least explain the delta: The fit image
> described in 161_fit.dts has four hash nodes, so with my estimate we
> first try with an increment of 4*128=512, while previously we'd first
> try 0 and then 1024. Since 512 is enough, the fit image ends up 512
> bytes smaller, and that is exactly the delta seen in fit:size and the
> other numbers affected by that.
> 
> Should I fold in those changes and resend, or is a separate followup
> patch better/ok?

Fold them in to a v2 please so we don't break bisectability of the
tests, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]