[PATCH v4 08/29] hash: integrate hash on mbedtls

[Simon Glass]

Hi Raymond,

On Thu, 18 Jul 2024 at 17:46, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> Hi Simon,
>
> On Fri, 5 Jul 2024 at 04:36, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi,
>>
>> On Wed, Jul 3, 2024, 09:56 Ilias Apalodimas <ilias.apalodimas at linaro.org> wrote:
>> >
>> > Hi Raymond
>> >
>> > On Tue, 2 Jul 2024 at 21:27, Raymond Mao <raymond.mao at linaro.org> wrote:
>> > >
>> > > Integrate common/hash.c on the hash shim layer so that hash APIs
>> > > from mbedtls can be leveraged by boot/image and efi_loader.
>> > >
>> > > Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
>> > > ---
>> > > Changes in v2
>> > > - Use the original head files instead of creating new ones.
>> > > Changes in v3
>> > > - Add handle checkers for malloc.
>> > > Changes in v4
>> > > - None.
>> > >
>> > >  common/hash.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> > >  1 file changed, 143 insertions(+)
>> > >
>> > > diff --git a/common/hash.c b/common/hash.c
>> > > index ac63803fed9..96caf074374 100644
>> > > --- a/common/hash.c
>> > > +++ b/common/hash.c
>> > > @@ -35,6 +35,141 @@
>> > >  #include <u-boot/sha512.h>
>> > >  #include <u-boot/md5.h>
>> > >
>> > > +#if CONFIG_IS_ENABLED(MBEDTLS_LIB_CRYPTO)
>> > > +
>> > > +static int hash_init_sha1(struct hash_algo *algo, void **ctxp)
>> > > +{
>> > > +       int ret;
>> > > +       mbedtls_sha1_context *ctx = malloc(sizeof(mbedtls_sha1_context));
>>
>>
>> Why do we need allocation here? We should avoid it where possible.
>>
> The API "hash_init_sha1(struct hash_algo *algo, void **ctxp)" is passing a pointer
> address and expecting to get the context from the pointer, it is reasonable to do the
> allocation.
> On top of that, this patch doesn't make changes on this API itself, but just adapted
> it to MbedTLS stacks, thus you can see the allocation is needed by the original API
> as well.

Oh dear., I see Now I am looking at the code. It is full of #ifdefs
for different cases.

The whole thing needs a bit of a rationalisation before adding another case.

Regards,
Simon