[U-Boot] [PATCH] drivers: optee: rpmb: fix returning CID to TEE

[Jorge Ramirez-Ortiz <jorge@foundries.io>]

On 11/18/19 10:36 AM, Jens Wiklander wrote:
> Hi Jorge,


hey!

>
> On Fri, Nov 15, 2019 at 10:37 PM Jorge Ramirez-Ortiz <jorge@foundries.io> wrote:
>> The MMC CID value is one of the input parameters to unequivocally
>> provision the the RPMB key.
>>
>> Before this patch, the value returned by the mmc driver in the Linux
>> kernel differs from the one returned by uboot to optee.
>>
>> This means that if Linux provisions the RPMB key, uboot wont be able
>> to access it (and the other way around).
>>
>> Fix it so both uboot and linux can access the RPMB partition
>> independently of who provisions the key.
>>
>> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
>> ---
>>  drivers/tee/optee/rpmb.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c
>> index 955155b3f8..5dbb1eae4a 100644
>> --- a/drivers/tee/optee/rpmb.c
>> +++ b/drivers/tee/optee/rpmb.c
>> @@ -98,6 +98,7 @@ static struct mmc *get_mmc(struct optee_private *priv, int dev_id)
>>  static u32 rpmb_get_dev_info(u16 dev_id, struct rpmb_dev_info *info)
>>  {
>>         struct mmc *mmc = find_mmc_device(dev_id);
>> +       int i;
>>
>>         if (!mmc)
>>                 return TEE_ERROR_ITEM_NOT_FOUND;
>> @@ -105,7 +106,9 @@ static u32 rpmb_get_dev_info(u16 dev_id, struct rpmb_dev_info *info)
>>         if (!mmc->ext_csd)
>>                 return TEE_ERROR_GENERIC;
>>
>> -       memcpy(info->cid, mmc->cid, sizeof(info->cid));
>> +       for (i = 0; i < ARRAY_SIZE(mmc->cid); i++)
>> +               ((u32 *) info->cid)[i] = be32_to_cpu(mmc->cid[i]);
>> +
> So it seems to be a byte order issue. I can't find the place in the
> Linux kernel (or in tee-supplicant) where the corresponding byte
> swapping is done. Have you been able to find it or you just tried to
> swap the bytes and it seemed to work?


I compared against the full CID output from Linux and noticed that in
order to match that exact same output this swap seemed to be required. I
didnt dig any deeper since a similar swap operation is done on other
-different - values returned from U-boot to OP-TEE.


>
> I'm not yet convinced that be32_to_cpu() is the correct function here.
> OP-TEE masks out a few fields from the CID when deriving the key:


sure but isnt that a different matter?

AFAICS U-boot should be providing the same CID than Linux does, and
whatever OP-TEE might be masking out on the receiving end is orthogonal
to such value, isnt it? maybe I am not understanding your point?


>
> CID is a uint8_t[16] here
>         /*
>          * PRV/CRC would be changed when doing eMMC FFU
>          * The following fields should be masked off when deriving RPMB key
>          *
>          * CID [55: 48]: PRV (Product revision)
>          * CID [07: 01]: CRC (CRC7 checksum)
>          * CID [00]: not used
>          */
>
> Will this work as expected on a big endian machine?
>
> Cheers,
> Jens
>
>>         info->rel_wr_sec_c = mmc->ext_csd[222];
>>         info->rpmb_size_mult = mmc->ext_csd[168];
>>         info->ret_code = RPMB_CMD_GET_DEV_INFO_RET_OK;
>> --
>> 2.23.0
>>