Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tee_shm_alloc: use a TEE_SHM_MAPPED shared memory as memref param #89

Open
omasse-linaro opened this issue Mar 10, 2021 · 3 comments
Open

Comments

@omasse-linaro
Copy link

omasse-linaro commented Mar 10, 2021

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
Copy link

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
Copy link
Author

omasse-linaro commented Mar 12, 2021

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants