Re: [Tee-dev] [libtomcrypt] Issue and patch in TEE adaptation for clearing big numbers

Hello Jens and Joachim,

Nice to read you again ! I hope your are fine.

I created a new pull request: https://github.com/OP-TEE/optee_os/pull/1171 I hope there is no grain of sand in my op.

Have a good afternoon Cheers Manu

-----Original Message----- From: Jens Wiklander [mailto:jens.wiklander@linaro.org] Sent: mercredi 9 novembre 2016 09:02 To: Emmanuel MICHEL emmanuel.michel@st.com Cc: tee-dev tee-dev@lists.linaro.org; Rodolphe DUBALD Rodolphe.DUBALD@st.com Subject: Re: [Tee-dev] [libtomcrypt] Issue and patch in TEE adaptation for clearing big numbers

Hi Manu,

Please make a pull request (at github) with this patch. If you know how to fix a problem, there's no need for an issue. A pull request with one or several commits fixing the problem and a description of the problem if it's not obvious is normally all that's needed to get the review process started.

Thanks, Jens

On Wed, Nov 9, 2016 at 6:51 AM, Emmanuel MICHEL emmanuel.michel@st.com wrote:

Hello all,

First time i propose a patch to OP-TEE and i do not know the process.

I try this channel.

I filled a ticket on github with all the info:

https://github.com/OP-TEE/optee_os/issues/1168

Best regards

Manu

Hello all,

It seems there is an issue in the TEE adaptation of the libtomcrypt for clearing big numbers.

Issue could be seen in the sequence:

TEE_AllocateTransientObject

TEE_ResetTransientObject

TEE_PopulateTransientObject

where TEE_ResetTransientObject clears (and not free) the object. Clear mean zero the data. If this object is a big number, the leaf function is:

static void bn_clear(struct bignum *s)

{

    struct mpa_numbase_struct *bn = (struct mpa_numbase_struct 

*)s;

    memset(bn, 0, bn->alloc);

}

corresponding to

static struct bignum *bn_allocate(size_t size_bits)

{

..

bn->alloc = sz - MPA_NUMBASE_METADATA_SIZE_IN_U32 * sizeof(uint32_t);

..

}

The underlying structure describing struct bignum is in lib/libmpa/include/mpalib.h:

typedef struct mpa_numbase_struct {

     mpa_asize_t alloc;

     mpa_usize_t size;

     mpa_word_t d[];

} mpa_num_base;

As we can see, memset fills with zero not only the data, but the metadata and the begin of the datas, instead of zero only the whole data.

Based on OP-TEE 2.2.0, even with the last commit 36d5a31, a proposed patch:

diff --git a/core/lib/libtomcrypt/src/tee_ltc_provider.c b/core/lib/libtomcrypt/src/tee_ltc_provider.c

index fda9454..69620c6 100644

--- a/core/lib/libtomcrypt/src/tee_ltc_provider.c

+++ b/core/lib/libtomcrypt/src/tee_ltc_provider.c

@@ -691,7 +691,7 @@ static void bn_clear(struct bignum *s)

{

    struct mpa_numbase_struct *bn = (struct mpa_numbase_struct 

*)s;

•   memset(bn, 0, bn->alloc);
  
  

•   memset(bn->d, 0, bn->alloc);
  
  

}

Note: mpa_numbase_struct.size is never set in this case, but it is not part of this ticket.

Best regards Manu

---

Tee-dev mailing list Tee-dev@lists.linaro.org https://lists.linaro.org/mailman/listinfo/tee-dev