Re: [PATCH v4] mmc: core: Use mrq.sbc in close-ended ffu

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

With help from Jens we turned tee-opplicant in userspace to single threaded with:

--- a/tee-supplicant/src/tee_supplicant.c
+++ b/tee-supplicant/src/tee_supplicant.c
@@ -588,6 +588,8 @@ static bool spawn_thread(struct thread_arg *arg)
        int e = 0;
        pthread_t tid;
 
+       return true;
+
        memset(&tid, 0, sizeof(tid));
 
        DMSG("Spawning a new thread");


but RPMB access still fails, so issue is not in userspace concurrency.
I added debug prints to this commit and the failures seem to come from
this first check idata->flags & MMC_BLK_IOC_DROP, second hunk in this patch:

@@ -475,10 +479,18 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
        unsigned int busy_timeout_ms;
        int err;
        unsigned int target_part;
+       struct mmc_blk_ioc_data *idata = idatas[i];
+       struct mmc_blk_ioc_data *prev_idata = NULL;

        if (!card || !md || !idata)
                return -EINVAL;

+       if (idata->flags & MMC_BLK_IOC_DROP)
+               return 0;
+
+       if (idata->flags & MMC_BLK_IOC_SBC)
+               prev_idata = idatas[i - 1];
+   
        /*
         * The RPMB accesses comes in from the character device, so we
         * need to target these explicitly. Else we just target the
 

Debug prints:

@@ -485,11 +485,19 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
        if (!card || !md || !idata)
                return -EINVAL;
 
-       if (idata->flags & MMC_BLK_IOC_DROP)
+       pr_err("%s: DEBUG0:\n", __func__ );
+
+       if (idata->flags & MMC_BLK_IOC_DROP) {
+               pr_err("%s: DEBUG1: idata->flags & MMC_BLK_IOC_DROP: flags = 0x%x, returning 0\n",
+                                                       __func__, idata->flags );
                return 0;
+       }
 
-       if (idata->flags & MMC_BLK_IOC_SBC)
+       if (idata->flags & MMC_BLK_IOC_SBC && i > 0) {
+               pr_err("%s: DEBUG2: idata->flags & MMC_BLK_IOC_SBC && i > 0, flags = 0x%x\n",
+                                                       __func__, idata->flags);
                prev_idata = idatas[i - 1];
+       }
 
        /*
         * The RPMB accesses comes in from the character device, so we

And the logs show that "idata->flags & MMC_BLK_IOC_DROP" is always true for the RPMB
ioctls.

https://ledge.validation.linaro.org/scheduler/job/83101

[   33.505035] __mmc_blk_ioctl_cmd: DEBUG0:
[   33.505426] __mmc_blk_ioctl_cmd: DEBUG1: idata->flags & MMC_BLK_IOC_DROP: flags = 0x5f797469, returning 0
[   33.506283] __mmc_blk_ioctl_cmd: DEBUG0:
[   33.506639] __mmc_blk_ioctl_cmd: DEBUG2: idata->flags & MMC_BLK_IOC_SBC && i > 0, flags = 0x702e796e
[   33.507447] __mmc_blk_ioctl_cmd: DEBUG2.1: RPMB
[   33.511746] __mmc_blk_ioctl_cmd: DEBUG3: copying to prev_idata
[   43.564084] mmc0: Card stuck being busy! __mmc_poll_for_busy

https://ledge.validation.linaro.org/scheduler/job/83102

[  143.124673] __mmc_blk_ioctl_cmd: DEBUG2: idata->flags & MMC_BLK_IOC_SBC && i > 0, flags = 0x485fb78a
[  143.133854] __mmc_blk_ioctl_cmd: DEBUG2.1: RPMB
[  143.138886] __mmc_blk_ioctl_cmd: DEBUG3: copying to prev_idata
...
[  153.166684] mmc0: Card stuck being busy! __mmc_poll_for_busy

This commit added uint flags to mmc_blk_ioc_data struct but it is only initialized for
MMC_DRV_OP_IOCTL code path and for MMC_DRV_OP_IOCTL_RPMB it is uninialized and happens to
be matching "& MMC_BLK_IOC_DROP" in all cases at runtime thus breaking RPMB ioctls.

Fix will be to initialize mmc_blk_ioc_data->flags in all cases. Would this be fine as
a catch all initialization for mmc_blk_ioc_data?

--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -413,7 +413,7 @@ static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user(
        struct mmc_blk_ioc_data *idata;
        int err;
 
-       idata = kmalloc(sizeof(*idata), GFP_KERNEL);
+       idata = kzalloc(sizeof(*idata), GFP_KERNEL);
        if (!idata) {
                err = -ENOMEM;
                goto out;

I think this is a sensible and future proof way to go.

Then, the second flags check for MMC_BLK_IOC_SBC is accessing array using
index i - 1, but is not checking if i == 0 which results in data[-1] access.
I think this should be fixed with something like:

-       if (idata->flags & MMC_BLK_IOC_SBC)
+       if (idata->flags & MMC_BLK_IOC_SBC && i > 0) {

Would you be fine with this?

With both of these changes in place (and debug logging) test runs on rockpi4b and synquacer
arm64 boards are now ok and firmware TPM devices with RPMB storage works again and optee fTPM TA does
not panic, though there is at least on TPM eventlog read test failing later on (a different kernel or
firmware bug, perhaps).

https://ledge.validation.linaro.org/scheduler/job/83094

+ tee-supplicant -d --rpmb-cid 7001004d33323530385212b201dea300
+ sleep 10
+ modprobe tpm_ftpm_tee
...
+ tpm2_createprimary -Q --hierarchy=o --key-context=prim.ctx
...
+ tpm2_loadexternal --key-algorithm=rsa --hierarchy=o --public=signing_key_public.pem --key-context=signing_key.ctx --name=signing_key.name
+ tpm2_startauthsession --session=session.ctx
+ tpm2_policyauthorize --session=session.ctx --policy=authorized.policy --name=signing_key.name
+ tpm2_flushcontext session.ctx
+ cat /tmp/rand_key
+ tpm2_create --hash-algorithm=sha256 --public=auth_pcr_seal_key.pub --private=auth_pcr_seal_key.priv --sealing-input=- --parent-context=prim.ctx --policy=authorized.policy
...
+ tpm2_load -Q --parent-context=prim.ctx --public=auth_pcr_seal_key.pub --private=auth_pcr_seal_key.priv --name=seal.name --key-context=seal.ctx
+ tpm2_evictcontrol -Q -C o -c 0x8100000a
...
+ cryptsetup -q --type luks2 --cipher aes-xts-plain --hash sha256 --use-random --uuid=6091b3a4-ce08-3020-93a6-f755a22ef03b luksFormat /dev/sda2 --key-file /tmp/rand_key --label otaroot
+ echo 'Creating encrypted filesystem ...'
Creating encrypted filesystem ...
+ cryptsetup luksOpen --key-file /tmp/rand_key /dev/sda2 rootfs
...

https://ledge.validation.linaro.org/scheduler/job/83096

+ modprobe tpm_ftpm_tee
...
+ rngd
...
+ tpm2_dictionarylockout -c
+ tpm2-abrmd --allow-root
...
+ tpm2_seal_password /tmp/rand_key
+ local passfilename=/tmp/rand_key
...
+ tpm2_createprimary -Q --hierarchy=o --key-context=prim.ctx
...
+ tpm2_loadexternal --key-algorithm=rsa --hierarchy=o --public=signing_key_public.pem --key-context=signing_key.ctx --name=signing_key.name
+ tpm2_startauthsession --session=session.ctx
+ tpm2_policyauthorize --session=session.ctx --policy=authorized.policy --name=signing_key.name
+ tpm2_flushcontext session.ctx
...
+ tpm2_create --hash-algorithm=sha256 --public=auth_pcr_seal_key.pub --private=auth_pcr_seal_key.priv --sealing-input=- --parent-context=prim.ctx --policy=authorized.policy
...
+ tpm2_load -Q --parent-context=prim.ctx --public=auth_pcr_seal_key.pub --private=auth_pcr_seal_key.priv --name=seal.name --key-context=seal.ctx
+ tpm2_evictcontrol -Q -C o -c 0x8100000a
+ tpm2_evictcontrol --hierarchy=o --object-context=seal.ctx 0x8100000a
...
+ cryptsetup -q --type luks2 --cipher aes-xts-plain --hash sha256 --use-random --uuid=6091b3a4-ce08-3020-93a6-f755a22ef03b luksFormat /dev/mmcblk1p7 --key-file /tmp/rand_key --label otaroot
...
Creating encrypted filesystem ...
+ cryptsetup luksOpen --key-file /tmp/rand_key /dev/mmcblk1p7 rootfs
...
+ mount /dev/mapper/rootfs /rootfs -o rw,noatime,iversion
[  171.018740] EXT4-fs (dm-0): mounted filesystem 89ae0ee0-b27c-4a66-ac0c-098c7ccd7a3c r/w with ordered data mode. Quota mode: disabled.
...

Cheers,

-Mikko




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux