lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20200805201628.GA11492@trex>
Date:   Wed, 5 Aug 2020 22:16:28 +0200
From:   "Jorge Ramirez-Ortiz, Foundries" <jorge@...ndries.io>
To:     Jens Wiklander <jens.wiklander@...aro.org>
Cc:     "Jorge Ramirez-Ortiz, Foundries" <jorge@...ndries.io>,
        sumit.garg@...aro.org, ricardo@...ndries.io, mike@...ndries.io,
        tee-dev@...ts.linaro.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv6] drivers: optee: allow op-tee to access devices on the
 i2c bus

On 05/08/20, Jens Wiklander wrote:
> On Wed, Aug 05, 2020 at 03:35:01PM +0200, Jorge Ramirez-Ortiz, Foundries wrote:
> > On 22/07/20, Jorge Ramirez-Ortiz wrote:
> > > Some secure elements like NXP's SE050 sit on I2C buses. For OP-TEE to
> > > control this type of cryptographic devices it needs coordinated access
> > > to the bus, so collisions and RUNTIME_PM dont get in the way.
> > > 
> > > This trampoline driver allow OP-TEE to access them.
> > > Tested on imx8mm LPDDR4
> > > 
> > > Signed-off-by: Jorge Ramirez-Ortiz <jorge@...ndries.io>
> > > ---
> > >  v6: compile out if CONFIG_I2C not enabled
> > >  v5: alphabetic order of includes
> > >  v4: remove unnecessary extra line in optee_msg.h
> > >  v3: use from/to msg param to support all types of memory
> > >      modify OPTEE_MSG_RPC_CMD_I2C_TRANSFER message id
> > >      
> > >  drivers/tee/optee/optee_msg.h | 16 +++++++
> > >  drivers/tee/optee/rpc.c       | 88 +++++++++++++++++++++++++++++++++++
> > >  2 files changed, 104 insertions(+)
> > > 
> > > diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h
> > > index 795bc19ae17a..14b580f55356 100644
> > > --- a/drivers/tee/optee/optee_msg.h
> > > +++ b/drivers/tee/optee/optee_msg.h
> > > @@ -419,4 +419,20 @@ struct optee_msg_arg {
> > >   */
> > >  #define OPTEE_MSG_RPC_CMD_SHM_FREE	7
> > >  
> > > +/*
> > > + * Access a device on an i2c bus
> > > + *
> > > + * [in]  param[0].u.value.a		mode: RD(0), WR(1)
> > > + * [in]  param[0].u.value.b		i2c adapter
> > > + * [in]  param[0].u.value.c		i2c chip
> > > + *
> > > + * [in/out] memref[1]			buffer to exchange the transfer data
> > > + *					with the secure world
> > > + *
> > > + * [out]  param[0].u.value.a		bytes transferred by the driver
> > > + */
> > > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER 21
> > > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD 0
> > > +#define OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR 1
> > > +
> > >  #endif /* _OPTEE_MSG_H */
> > > diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> > > index b4ade54d1f28..5fd5c6c93896 100644
> > > --- a/drivers/tee/optee/rpc.c
> > > +++ b/drivers/tee/optee/rpc.c
> > > @@ -7,6 +7,7 @@
> > >  
> > >  #include <linux/delay.h>
> > >  #include <linux/device.h>
> > > +#include <linux/i2c.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/tee_drv.h>
> > >  #include "optee_private.h"
> > > @@ -49,6 +50,90 @@ static void handle_rpc_func_cmd_get_time(struct optee_msg_arg *arg)
> > >  	arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > >  }
> > >  
> > > +#if IS_ENABLED(CONFIG_I2C)
> > > +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> > > +					     struct optee_msg_arg *arg)
> > > +{
> > > +	struct i2c_client client;
> > > +	struct tee_param *params;
> > > +	uint32_t type;
> > > +	int i, ret;
> > > +	size_t len;
> > > +	char *buf;
> > > +	uint32_t attr[] = {
> > > +		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT,
> > > +		TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT,
> > > +		TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_OUTPUT,
> > > +	};
> > > +
> > > +	if (arg->num_params != ARRAY_SIZE(attr)) {
> > > +		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > +		return;
> > > +	}
> > > +
> > > +	params = kmalloc_array(arg->num_params, sizeof(struct tee_param),
> > > +			       GFP_KERNEL);
> > > +	if (!params) {
> > > +		arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
> > > +		return;
> > > +	}
> > > +
> > > +	if (optee_from_msg_param(params, arg->num_params, arg->params))
> > > +		goto bad;
> > > +
> > > +	for (i = 0; i < arg->num_params; i++) {
> > > +		type = params[i].attr & TEE_IOCTL_PARAM_ATTR_TYPE_MASK;
> > > +		if (type != attr[i])
> > > +			goto bad;
> > > +	}
> > > +
> > > +	client.addr = params[0].u.value.c;
> > > +	client.adapter = i2c_get_adapter(params[0].u.value.b);
> > > +	if (!client.adapter)
> > > +		goto bad;
> > > +
> > > +	snprintf(client.name, I2C_NAME_SIZE, "i2c%d", client.adapter->nr);
> > > +
> > > +	buf = params[1].u.memref.shm->kaddr;
> > > +	len = params[1].u.memref.size;
> > > +
> > > +	switch (params[0].u.value.a) {
> > > +	case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
> > > +		ret = i2c_master_recv(&client, buf, len);
> > > +		break;
> > > +	case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
> > > +		ret = i2c_master_send(&client, buf, len);
> > > +		break;
> > > +	default:
> > > +		i2c_put_adapter(client.adapter);
> > > +		goto bad;
> > > +	}
> > > +
> > > +	if (ret >= 0) {
> > > +		params[2].u.value.a = ret;
> > > +		arg->ret = TEEC_SUCCESS;
> > > +	} else {
> > > +		arg->ret = TEEC_ERROR_COMMUNICATION;
> > > +	}
> > > +
> > > +	if (optee_to_msg_param(arg->params, arg->num_params, params))
> > > +		arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > +
> > > +	i2c_put_adapter(client.adapter);
> > > +	kfree(params);
> > > +	return;
> > > +bad:
> > > +	kfree(params);
> > > +	arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > +}
> > > +#else
> > > +static void handle_rpc_func_cmd_i2c_transfer(struct tee_context *ctx,
> > > +					     struct optee_msg_arg *arg)
> > > +{
> > > +	arg->ret = TEEC_ERROR_COMMUNICATION;
> > > +}
> > > +#endif
> > > +
> > >  static struct wq_entry *wq_entry_get(struct optee_wait_queue *wq, u32 key)
> > >  {
> > >  	struct wq_entry *w;
> > > @@ -382,6 +467,9 @@ static void handle_rpc_func_cmd(struct tee_context *ctx, struct optee *optee,
> > >  	case OPTEE_MSG_RPC_CMD_SHM_FREE:
> > >  		handle_rpc_func_cmd_shm_free(ctx, arg);
> > >  		break;
> > > +	case OPTEE_MSG_RPC_CMD_I2C_TRANSFER:
> > > +		handle_rpc_func_cmd_i2c_transfer(ctx, arg);
> > > +		break;
> > >  	default:
> > >  		handle_rpc_supp_cmd(ctx, arg);
> > >  	}
> > 
> > 
> > any comments please?
> 
> As you know we're still reviewing the secure world counterpart at
> https://github.com/OP-TEE/optee_os/pull/3905

yep

> Where we're sorting out the ABI. Thanks for your patience.

after the initial comments on the first patch it was not clear why
there were none on the follow up ones. but sure, thanks for taking the
time.


> 
> Cheers,
> Jens

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ