Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1332252 > unrolled thread

[PATCH v8 0/4] generic TEE subsystem

Started byJens Wiklander <jens.wiklander@linaro.org>
First post2016-02-11 18:20 +0100
Last post2016-02-12 12:40 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v8 0/4] generic TEE subsystem Jens Wiklander <jens.wiklander@linaro.org> - 2016-02-11 18:20 +0100
    Re: [PATCH v8 2/4] tee: generic TEE subsystem Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-02-12 04:20 +0100
    Re: [PATCH v8 2/4] tee: generic TEE subsystem Al Viro <viro@ZenIV.linux.org.uk> - 2016-02-12 06:00 +0100
      Re: [PATCH v8 2/4] tee: generic TEE subsystem Jens Wiklander <jens.wiklander@linaro.org> - 2016-02-12 12:40 +0100

#1332252 — [PATCH v8 0/4] generic TEE subsystem

FromJens Wiklander <jens.wiklander@linaro.org>
Date2016-02-11 18:20 +0100
Subject[PATCH v8 0/4] generic TEE subsystem
Message-ID<r134m-3on-13@gated-at.bofh.it>
Hi,

This patch set introduces a generic TEE subsystem. The TEE subsystem will
contain drivers for various TEE implementations. A TEE (Trusted Execution
Environment) is a trusted OS running in some secure environment, for
example, TrustZone on ARM CPUs, or a separate secure co-processor etc.

Regarding use cases, TrustZone has traditionally been used for
offloading secure tasks to the secure world. Examples include: 
- Secure key handling where the OS may or may not have direct access to key
  material.
- E-commerce and payment technologies. Credentials, credit card numbers etc
  could be stored in a more secure environment.
- Trusted User Interface (TUI) to ensure that no-one can snoop PIN-codes
  etc.
- Secure boot to ensure that loaded binaries haven’t been tampered with.
  It’s not strictly needed for secure boot, but you could enhance security
  by leveraging a TEE during boot.
- Digital Rights Management (DRM), the studios provides content with
  different resolution depending on the security of the device. Higher
  security means higher resolution.

A TEE could also be used in existing and new technologies. For example IMA
(Integrity Measurement Architecture) which has been in the kernel for quite
a while. Today you can enhance security by using a TPM-chip to sign the IMA
measurement list. This is something that you also could do by leveraging a
TEE.

Another example could be in 2-factor authentication which is becoming
increasingly more important. FIDO (https://fidoalliance.org) for example
are using public key cryptography in their 2-factor authentication standard
(U2F). With FIDO, a private and public key pair will be generated for every
site you visit and the private key should never leave the local device.
This is an example where you could use secure storage in a TEE for the
private key.

Today you will find a quite a few different out of tree implementations of
TEE drivers which tends to fragment the TEE ecosystem and development. We
think it would be a good idea to have a generic TEE driver integrated in
the kernel which would serve as a base for several different TEE solutions,
no matter if they are on-chip like TrustZone or if they are on a separate
crypto co-processor.

To develop this TEE subsystem we have been using the open source TEE called
OP-TEE (https://github.com/OP-TEE/optee_os) and therefore this would be the
first TEE solution supported by this new subsystem. OP-TEE is a
GlobalPlatform compliant TEE, however this TEE subsystem is not limited to
only GlobalPlatform TEEs, instead we have tried to design it so that it
should work with other TEE solutions also.

"tee: generic TEE subsystem" brings in the generic TEE subsystem which
helps when writing a driver for a specific TEE, for example, OP-TEE.

"tee: add OP-TEE driver" is an OP-TEE driver which uses the subsystem to do
its work.

This patch set has been prepared in cooperation with Javier González who
proposed "Generic TrustZone Driver in Linux Kernel" patches 28 Nov 2014,
https://lwn.net/Articles/623380/ . We've since then changed the scope to
TEE instead of TrustZone.

We have discussed the design on tee-dev@lists.linaro.org (archive at
https://lists.linaro.org/pipermail/tee-dev/) with people from other
companies, including Valentin Manea <valentin.manea@huawei.com>,
Emmanuel MICHEL <emmanuel.michel@st.com>,
Jean-michel DELORME <jean-michel.delorme@st.com>,
and Joakim Bech <joakim.bech@linaro.org>. Our main concern has been to
agree on something that is generic enough to support many different
TEEs while still keeping the interface together.

v8:
* Rebased on v4.5-rc3
* dt/bindings: add bindings for optee
  Acked-by: Rob Herring <robh@kernel.org>
* Fixes build error for X86
* Fixes spell error in "dt/bindings: add bindings for optee"

v7:
* Rebased on v4.5-rc2
* Moved the ARM SMC Calling Convention support into a separate patch
  set, which is now merged

v6:
* Rebased on v4.3-rc7
* Changed smccc interface to let the compiler marshal most of the
  parameters
* Added ARCH64 capability for smccc interface
* Changed the PSCI firmware calls (both arm and arm64) to use the new
  generic smccc interface instead instead of own assembly functions.
* Move optee DT bindings to below arm/firmware
* Defines method for OP-TEE driver to call secure world in DT, smc or hvc
* Exposes implementation id of a TEE driver in sysfs
  to easily spawn corresponding tee-supplicant when device is ready
* Update OP-TEE Message Protocol to better cope with fragmented physical
  memory
* Read time directly from OP-TEE driver instead of forwarding the RPC
  request to tee-supplicant

v5:
* Replaced kref reference counting for the device with a size_t instead as
  the counter is always protected by a mutex

v4:
* Rebased on 4.1
* Redesigned the synchronization around entry exit of normal SMC
* Replaced rwsem on the driver instance with kref and completion since
  rwsem wasn't intended to be used in this way
* Expanded the TEE_IOCTL_PARAM_ATTR_TYPE_MASK to make room for
  future additional parameter types
* Documents TEE subsystem and OP-TEE driver
* Replaced TEE_IOC_CMD with TEE_IOC_OPEN_SESSION, TEE_IOC_INVOKE,
  TEE_IOC_CANCEL and TEE_IOC_CLOSE_SESSION
* DT bindings in a separate patch
* Assembly parts moved to arch/arm and arch/arm64 respectively, in a
  separate patch
* Redefined/clarified the meaning of OPTEE_SMC_SHM_CACHED
* Removed CMA usage to limit the scope of the patch set

v3:
* Rebased on 4.1-rc3 (dma_buf_export() API change)
* A couple of small sparse fixes
* Documents bindings for OP-TEE driver
* Updated MAINTAINERS

v2:
* Replaced the stubbed OP-TEE driver with a real OP-TEE driver
* Removed most APIs not needed by OP-TEE in current state
* Update Documentation/ioctl/ioctl-number.txt with correct path to tee.h
* Rename tee_shm_pool_alloc_cma() to tee_shm_pool_alloc()
* Moved tee.h into include/uapi/linux/
* Redefined tee.h IOCTL macros to be directly based on _IOR and friends
* Removed version info on the API to user space, a data blob which
  can contain an UUID is left for user space to be able to tell which
  protocol to use in TEE_IOC_CMD
* Changed user space exposed structures to only have types with __ prefix
* Dropped THIS_MODULE from tee_fops
* Reworked how the driver is registered and ref counted:
  - moved from using an embedded struct miscdevice to an embedded struct
    device.
  - uses an struct rw_semaphore as synchronization for driver detachment
  - uses alloc/register pattern from TPM

Thanks,
Jens



Jens Wiklander (4):
  dt/bindings: add bindings for optee
  tee: generic TEE subsystem
  tee: add OP-TEE driver
  Documentation: tee subsystem and op-tee driver

 Documentation/00-INDEX                             |   2 +
 .../bindings/arm/firmware/linaro,optee-tz.txt      |  31 +
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 Documentation/ioctl/ioctl-number.txt               |   1 +
 Documentation/tee.txt                              | 117 +++
 MAINTAINERS                                        |  13 +
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/tee/Kconfig                                |  19 +
 drivers/tee/Makefile                               |   4 +
 drivers/tee/optee/Kconfig                          |   8 +
 drivers/tee/optee/Makefile                         |   5 +
 drivers/tee/optee/call.c                           | 400 ++++++++++
 drivers/tee/optee/core.c                           | 522 ++++++++++++
 drivers/tee/optee/optee_msg.h                      | 423 ++++++++++
 drivers/tee/optee/optee_private.h                  | 146 ++++
 drivers/tee/optee/optee_smc.h                      | 418 ++++++++++
 drivers/tee/optee/rpc.c                            | 322 ++++++++
 drivers/tee/optee/supp.c                           | 212 +++++
 drivers/tee/tee.c                                  | 873 +++++++++++++++++++++
 drivers/tee/tee_private.h                          |  80 ++
 drivers/tee/tee_shm.c                              | 324 ++++++++
 drivers/tee/tee_shm_pool.c                         | 133 ++++
 include/linux/tee_drv.h                            | 299 +++++++
 include/uapi/linux/tee.h                           | 383 +++++++++
 25 files changed, 4739 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/linaro,optee-tz.txt
 create mode 100644 Documentation/tee.txt
 create mode 100644 drivers/tee/Kconfig
 create mode 100644 drivers/tee/Makefile
 create mode 100644 drivers/tee/optee/Kconfig
 create mode 100644 drivers/tee/optee/Makefile
 create mode 100644 drivers/tee/optee/call.c
 create mode 100644 drivers/tee/optee/core.c
 create mode 100644 drivers/tee/optee/optee_msg.h
 create mode 100644 drivers/tee/optee/optee_private.h
 create mode 100644 drivers/tee/optee/optee_smc.h
 create mode 100644 drivers/tee/optee/rpc.c
 create mode 100644 drivers/tee/optee/supp.c
 create mode 100644 drivers/tee/tee.c
 create mode 100644 drivers/tee/tee_private.h
 create mode 100644 drivers/tee/tee_shm.c
 create mode 100644 drivers/tee/tee_shm_pool.c
 create mode 100644 include/linux/tee_drv.h
 create mode 100644 include/uapi/linux/tee.h

-- 
1.9.1

[toc] | [next] | [standalone]

#1332502 — Re: [PATCH v8 2/4] tee: generic TEE subsystem

FromGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Date2016-02-12 04:20 +0100
SubjectRe: [PATCH v8 2/4] tee: generic TEE subsystem
Message-ID<r1cr0-1gv-3@gated-at.bofh.it>
In reply to#1332252
On Thu, Feb 11, 2016 at 06:14:35PM +0100, Jens Wiklander wrote:
> Initial patch for generic TEE subsystem.
> This subsystem provides:
> * Registration/un-registration of TEE drivers.
> * Shared memory between normal world and secure world.
> * Ioctl interface for interaction with user space.
> * Sysfs implementation_id of TEE driver
> 
> A TEE (Trusted Execution Environment) driver is a driver that interfaces
> with a trusted OS running in some secure environment, for example,
> TrustZone on ARM cpus, or a separate secure co-processor etc.
> 
> The TEE subsystem can serve a TEE driver for a Global Platform compliant
> TEE, but it's not limited to only Global Platform TEEs.
> 
> This patch builds on other similar implementations trying to solve
> the same problem:
> * "optee_linuxdriver" by among others
>   Jean-michel DELORME<jean-michel.delorme@st.com> and
>   Emmanuel MICHEL <emmanuel.michel@st.com>
> * "Generic TrustZone Driver" by Javier González <javier@javigon.com>
> 
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>

I need you to get other people to review this before I can accept this
series.  Please do that, surely someone else cares about this code
becides you?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1332533 — Re: [PATCH v8 2/4] tee: generic TEE subsystem

FromAl Viro <viro@ZenIV.linux.org.uk>
Date2016-02-12 06:00 +0100
SubjectRe: [PATCH v8 2/4] tee: generic TEE subsystem
Message-ID<r1dZM-25w-9@gated-at.bofh.it>
In reply to#1332252
On Thu, Feb 11, 2016 at 06:14:35PM +0100, Jens Wiklander wrote:

> +static int tee_ioctl_shm_alloc(struct tee_context *ctx,
> +		struct tee_ioctl_shm_alloc_data __user *udata)
> +{
> +	long ret;
> +	struct tee_ioctl_shm_alloc_data data;
> +	struct tee_shm *shm;
> +
> +	if (copy_from_user(&data, udata, sizeof(data)))
> +		return -EFAULT;
> +
> +	/* Currently no input flags are supported */
> +	if (data.flags)
> +		return -EINVAL;
> +
> +	data.fd = -1;
> +
> +	shm = tee_shm_alloc(ctx->teedev, data.size,
> +			    TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> +	if (IS_ERR(shm))
> +		return PTR_ERR(shm);
> +
> +	data.flags = shm->flags;
> +	data.size = shm->size;
> +	data.fd = tee_shm_get_fd(shm);
> +	if (data.fd < 0) {
> +		ret = data.fd;
> +		goto err;
> +	}
> +
> +	if (copy_to_user(udata, &data, sizeof(data))) {
> +		ret = -EFAULT;
> +		goto err;
> +	}
> +	/*
> +	 * When user space closes the file descriptor the shared memory
> +	 * should be freed
> +	 */
> +	tee_shm_put(shm);
> +	return 0;
> +err:
> +	if (data.fd >= 0)
> +		tee_shm_put_fd(data.fd);

This is completely broken.  Don't ever use that pattern.  Once something
is in descriptor table, that's _it_.  You are already past the point of
no return and there is no way to clean up.

In ABIs like that (and struct containing descriptor *is* a bad ABI design)
solution is
	* allocate a descriptor
	* do everything that might fail, including copy_to_user()/put_user(),
etc.
	* if failed, release unused descriptor and do fput(), if you already
have a struct file reference that needs to be released.
	* FINALLY, when nothing no failures are possible, fd_install() the
sucker in place.

And yes, dma_buf_fd() encourages that kind of braindamage.  It's tolerable
only in one case - when we are about to return descriptor number directly
as return value of syscall and really can't fail anymore.  Not the case
here.

[toc] | [prev] | [next] | [standalone]

#1332688 — Re: [PATCH v8 2/4] tee: generic TEE subsystem

FromJens Wiklander <jens.wiklander@linaro.org>
Date2016-02-12 12:40 +0100
SubjectRe: [PATCH v8 2/4] tee: generic TEE subsystem
Message-ID<r1keR-6lx-5@gated-at.bofh.it>
In reply to#1332533
On Fri, Feb 12, 2016 at 04:51:59AM +0000, Al Viro wrote:
> On Thu, Feb 11, 2016 at 06:14:35PM +0100, Jens Wiklander wrote:
> 
> > +static int tee_ioctl_shm_alloc(struct tee_context *ctx,
> > +		struct tee_ioctl_shm_alloc_data __user *udata)
> > +{
> > +	long ret;
> > +	struct tee_ioctl_shm_alloc_data data;
> > +	struct tee_shm *shm;
> > +
> > +	if (copy_from_user(&data, udata, sizeof(data)))
> > +		return -EFAULT;
> > +
> > +	/* Currently no input flags are supported */
> > +	if (data.flags)
> > +		return -EINVAL;
> > +
> > +	data.fd = -1;
> > +
> > +	shm = tee_shm_alloc(ctx->teedev, data.size,
> > +			    TEE_SHM_MAPPED | TEE_SHM_DMA_BUF);
> > +	if (IS_ERR(shm))
> > +		return PTR_ERR(shm);
> > +
> > +	data.flags = shm->flags;
> > +	data.size = shm->size;
> > +	data.fd = tee_shm_get_fd(shm);
> > +	if (data.fd < 0) {
> > +		ret = data.fd;
> > +		goto err;
> > +	}
> > +
> > +	if (copy_to_user(udata, &data, sizeof(data))) {
> > +		ret = -EFAULT;
> > +		goto err;
> > +	}
> > +	/*
> > +	 * When user space closes the file descriptor the shared memory
> > +	 * should be freed
> > +	 */
> > +	tee_shm_put(shm);
> > +	return 0;
> > +err:
> > +	if (data.fd >= 0)
> > +		tee_shm_put_fd(data.fd);
> 
> This is completely broken.  Don't ever use that pattern.  Once something
> is in descriptor table, that's _it_.  You are already past the point of
> no return and there is no way to clean up.
> 
> In ABIs like that (and struct containing descriptor *is* a bad ABI design)
> solution is
> 	* allocate a descriptor
> 	* do everything that might fail, including copy_to_user()/put_user(),
> etc.
> 	* if failed, release unused descriptor and do fput(), if you already
> have a struct file reference that needs to be released.
> 	* FINALLY, when nothing no failures are possible, fd_install() the
> sucker in place.
> 
> And yes, dma_buf_fd() encourages that kind of braindamage.  It's tolerable
> only in one case - when we are about to return descriptor number directly
> as return value of syscall and really can't fail anymore.  Not the case
> here.

Thanks for the feedback, I'll change to return the descriptor in the
return value instead.

--
Jens

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web