tee_shm_alloc: use a TEE_SHM_MAPPED shared memory as memref param

[omasse-linaro]

Hi @jenswi-linaro ,

We've faced an issue with tee_shm_alloc kernel API when migrating from optee 3.7 to 3.10. Our driver was using a shared memory allocated as follow: shm = tee_shm_alloc(ctx,size,TEE_SHM_MAPPED);

which was used in parameter of tee_client_invoke_func: param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT; param[1].u.memref.shm_offs = offset; param[1].u.memref.size = size; param[1].u.memref.shm = shm;

but it seems that it is no more possible with optee 3.10 and we have to allocate shared memory with TEE_SHM_DMA_BUF flag. without doing that set_tmem_param return TEE_ERROR_BAD_PARAMETERS because our buffer is not in the contiguous shared memory (or OPTEE static shared memory).

However tee_shm_alloc will allocate into the dynamic shared memory area without the OPTEE_MSG_ATTR_NONCONTIG attribute.

Could you tell me if it is a restriction or not ?

Best regards, Olivier

[jenswi-linaro]

Hi @omasse-linaro

I can't find anything obvious. Would it be possible to bisect optee_os to see which change caused this? Or do you have an easy test case I could reproduce this with on QEMU?

Thanks, Jens

[omasse-linaro]

Hi @jenswi-linaro

My driver unit test is as follow. Please use another TA or PTA to open a session and calling invoke.

void hantro_secure_unit_test(void)
{
    int ret = 0;
    const RTC_UUID pta_uuid = PTA_HANTRO_VPU_PTA_UUID;
    struct tee_ioctl_open_session_arg sess_arg = { 0 };
    struct tee_ioctl_invoke_arg inv_arg = { 0 };
    struct tee_param param[4] = { 0 };
    struct tee_param *params = NULL;
    struct tee_context *ctx;
    struct tee_shm* shm;

    struct tee_ioctl_version_data vers = {
        .impl_id = TEE_OPTEE_CAP_TZ,
        .impl_caps = TEE_IMPL_ID_OPTEE,
        .gen_caps = TEE_GEN_CAP_GP,
    };

    ctx = tee_client_open_context(NULL, hantrodec_optee_match,
                         NULL, &vers);

    if (IS_ERR(ctx))
    {
        ret = -EINVAL;
        pr_err("unable to open tee ctx %p\n",(void*)ctx);
        goto err_ctx;
    }

    /* Open session with pseudo TA */
    uuid_to_octets(sess_arg.uuid, &pta_uuid);
    sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
    sess_arg.num_params = 0;

    ret = tee_client_open_session(ctx, &sess_arg, params);
    if ((ret < 0) || sess_arg.ret) {
        pr_err("unable to open pta session 0x%08X\n",sess_arg.ret);
        goto err_sess;
    }

    shm = tee_shm_alloc(ctx,1024,TEE_SHM_MAPPED);
    if (!shm) {
        ret = -EINVAL;
        pr_err("unable to allocate shm\n");
        goto err_shm;
    }

    /* Invoke PTA_HANTRO_VPU_CMD_READ_MULTIPLE function */
    inv_arg.func = PTA_HANTRO_VPU_CMD_READ_MULTIPLE;
    inv_arg.session = sess_arg.session;
    inv_arg.num_params = 4;

    /* Fill invoke cmd params */
    param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT;
    param[0].u.value.a = 0;
    param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
    param[1].u.memref.shm_offs = 0;
    param[1].u.memref.size = 1024;
    param[1].u.memref.shm = shm;

    ret = tee_client_invoke_func(ctx, &inv_arg, param);
    if ((ret < 0) || inv_arg.ret) {
        pr_err("PTA_HANTRO_VPU_CMD_READ_MULTIPLE invoke function err: 0x%08X 0x%08X %d\n",
               ret,inv_arg.ret,inv_arg.ret_origin);
        ret = -EINVAL;
    }

    tee_shm_free(shm);
err_shm:
    tee_client_close_session(ctx,sess_arg.session);
err_sess:
    tee_client_close_context(ctx);
err_ctx:
    if (ret < 0)
        pr_info("hantro_secure_unit_test FAILED\n");
    else
        pr_info("hantro_secure_unit_test SUCCESS\n");
}

log result is: [ 3.777302] PTA_HANTRO_VPU_CMD_READ_MULTIPLE invoke function err: 0x00000000 0xFFFF0006 3

which is TEE_ERROR_BAD_PARAMETERS from TEE_ORIGIN_TEE.

BR / Olivier

[jenswi-linaro]

I tried something similar, but this doesn't even work with 3.7.0. The TEE_SHM_DMA_BUF causes the shared memory to be registered which is needed for memory which doesn't come from the carved out shared memory pool. We could perhaps extend the OP-TEE driver to manage this without this flag since the dma-buf part of this is quite useless for kernel only buffers.

However, for now we need the TEE_SHM_DMA_BUF for shm buffers passed with the in-kernel client api.