Skip to content

Optee rev #54

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

Open
wants to merge 45 commits into
base: optee
Choose a base branch
from
Open

Optee rev #54

wants to merge 45 commits into from

Conversation

jforissier
Copy link

No description provided.

jenswi-linaro and others added 30 commits August 21, 2017 13:35
Fixes the static checker warning in optee_release().
error: uninitialized symbol 'parg'.

Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
tee_drv.h references struct device, but does not include device.h nor
platform_device.h. Therefore, if tee_drv.h is included by some file
that does not pull device.h nor platform_device.h beforehand, we have a
compile warning. Fix this by adding a forward declaration.

Signed-off-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
dma_buf_ops are not supposed to change at runtime. All functions
working with dma_buf_ops provided by <linux/dma-buf.h> work with
const dma_buf_ops. So mark the non-const structs as const.

File size before:
   text	   data	    bss	    dec	    hex	filename
   2026	    112	      0	   2138	    85a	drivers/tee/tee_shm.o

File size After adding 'const':
   text	   data	    bss	    dec	    hex	filename
   2138	      0	      0	   2138	    85a	drivers/tee/tee_shm.o

Signed-off-by: Arvind Yadav <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Add const to tee_desc structures as they are only passed as an argument
to the function tee_device_alloc. This argument is of type const, so
declare these structures as const too.
Add const to tee_driver_ops structures as they are only stored in the
ops field of a tee_desc structure. This field is of type const, so
declare these structure types as const.

Signed-off-by: Bhumika Goyal <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Prior to this patch RPC sleep was uninterruptible since msleep() is
uninterruptible. Change to use msleep_interruptible() instead.

Signed-off-by: Tiger Yu <[email protected]>
Reviewed-by: Joakim Bech <[email protected]>
Signed-off-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Mirrors the TEE_DESC_PRIVILEGED bit of struct tee_desc:flags into struct
tee_ioctl_version_data:gen_caps as TEE_GEN_CAP_PRIVILEGED in
tee_ioctl_version()

Reviewed-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
In the latest changes of optee_os, the interrupts' names are
changed to "native" and "foreign" interrupts.

Signed-off-by: David Wang <[email protected]>
Signed-off-by: Jerome Forissier <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Adds a kernel internal TEE client interface to be used by other drivers.

Signed-off-by: Jens Wiklander <[email protected]>
From the commit below, the mt8173-evb failed to boot to console due to
changes in the mt8173 device tree files.

  commit c0d6fe2
  Merge: b44a3d2 3e4dda7
  Author: Linus Torvalds <[email protected]>
  Date:   Tue Nov 10 15:06:26 2015 -0800

      Merge tag 'armsoc-dt' of
      git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc

Until properly solved, let's just remove the section in the device tree
blob that causes this.

Signed-off-by: Joakim Bech <[email protected]>
Reviewed-by: Pascal Brand <[email protected]>
Enables use of PSCI for foundation-v8.

Signed-off-by: Jens Wiklander <[email protected]>
Uses GICv2 for foundation-v8.

Signed-off-by: Jens Wiklander <[email protected]>
Configures foundation-v8 with OP-TEE.

Signed-off-by: Jens Wiklander <[email protected]>
Configures Juno with OP-TEE.

Reviewed-by: Pascal Brand <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
…dation-v8 **not for mainline**

All the platforms that reserve memory for OP-TEE statically via the
DT (i.e., not those that reserve it via UEFI or that patch the DT
dynamically thanks to OP-TEE's CFG_DT option) have to mark it 'no-map'
so that only the TEE driver may map it.

Signed-off-by: Jens Wiklander <[email protected]>
Prior to this patch the ARM_SMCCC_FAST_CALL constant was of an unsigned
type causing unwanted sign extension. This patch explicitly selects an
unsigned type for the constant.

Reviewed-by: Pascal Brand <[email protected]>
Tested-by: Jens Wiklander <[email protected]> (QEMU Aarch64)
Reported-by: Saksham Jain <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
This change allows userland to create a tee_shm object that refers
to a dmabuf reference.

Userland provides a dmabuf file descriptor as buffer reference.
The created tee_shm object exported as a brand new dmabuf reference
used to provide a clean fd to userland. Userland shall closed this new
fd to release the tee_shm object resources. The initial dmabuf resources
are tracked independently through original dmabuf file descriptor.

Once the buffer is registered and until it is released, TEE driver
keeps a refcount on the registered dmabuf structure.

This change only support dmabuf references that relates to physically
contiguous memory buffers.

New tee_shm flag to identify tee_shm objects built from a registered
dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged both
TEE_SHM_DMA_BUF and TEE_SHM_EXT_DMA_BUF.

Signed-off-by: Etienne Carriere <[email protected]>
Reviewed-by: Jens Wiklander <[email protected]>
… **not for mainline**

All the platforms that reserve memory for OP-TEE statically via the
DT (i.e., not those that reserve it via UEFI or that patch the DT
dynamically thanks to OP-TEE's CFG_DT option) have to mark it 'no-map'
so that only the TEE driver may map it.

Signed-off-by: Jens Wiklander <[email protected]>
Signed-off-by: Joakim Bech <[email protected]>
Reviewed-by: Pascal Brand <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
Reviewed-by: Etienne Carriere <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Adds TEE_IOCTL_PARAM_ATTR_META with can be used to indicate meta
parameters when communicating with user space. These meta parameters can
be used by supplicant support multiple parallel requests at a time.

Reviewed-by: Etienne Carriere <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
Adds support for asynchronous supplicant requests, meaning that the
supplicant can process several requests in parallel or block in a
request for some time.

Acked-by: Etienne Carriere <[email protected]>
Tested-by: Etienne Carriere <[email protected]> (b2260 pager=y/n)
Signed-off-by: Jens Wiklander <[email protected]>
Add Benchmark support

Reviewed-by: Joakim Bech <[email protected]>
Signed-off-by: Igor Opaniuk <[email protected]>
<linux/tee_drv.h> is not used by the benchmark code and happens to
introduce a compile warning if it is included without
<linux/platform_device.h> (or, more exactly, <linux/device.h>).

In file included from drivers/tee/optee/optee_bench.h:19:0,
                 from drivers/tee/optee/bench.c:15:
./include/linux/tee_drv.h:127:16: warning: 'struct device' declared inside parameter list will not be visible outside of this definition or declaration
         struct device *dev,
                ^~~~~~

Fixes: 4867f93 ("OP-TEE Benchmark **not for mainline**")
Signed-off-by: Jerome Forissier <[email protected]>
Reviewed-by: Joakim Bech <[email protected]>
Acked-by: Jens Wiklander <[email protected]>
Dumped from:
  https://github.com/loboris/OrangePI-Kernel/tree/master/linux-3.4
  0cc8d855adb457d1860d6e25cb93b6cc75d5a09d
  Author: Sunny <[email protected]> for Allwinner.

Changes made on original "secure heap" implementation:
- minor coding style: fix includes, empty lines and overlong lines,
  indentation, comment layout.
- Original path modified the ion uapi. We do not attempt to modify
  uapi/ion.h. "secure" (or "domain") heaps are under ID
  ION_HEAP_TYPE_CUSTOM + 1 (legacy 'secure heap type' value).

Signed-off-by: Etienne Carriere <[email protected]>
OP-TEE/SDP (Secure Data Path) memory pools are created through ION
secure type heap" from Allwinner. This change renames "secure" into
"unmapped" as, from Linux point of view, the heap constraint is
manipulating unampped memory pools/buffers.

"Unmapped" heap support is integrated in ION UAPI (actually this was
the Allwinner initial proposal) and ION DT parsing support.

Based in work from Sunny <[email protected]> for Allwinner.

Changes:
- rename "secure_heap" into "unmapped_heap"
- define ION_HEAP_TYPE_UNMAPPED in ION UAPI (sic!)
- add structure "struct unmapped_buffer_priv" to hold allocated buffer
  private data (currently only the buffer physical address.
- adapt to recent ION (i.e ion_phys_addr_t => phys_addr_t)
- Support dummy heap configuration: one can hard code into the Linux
  kernel configuration the location of a "unmapped heap". It will be
  created during ION device inits: see CONFIG_ION_DUMMY_UNMAPPED_HEAP.

Signed-off-by: Etienne Carriere <[email protected]>
Condition ION unmapped heap implementation to architectures that
currently support it. ARM is one of these.

Signed-off-by: Etienne Carriere <[email protected]>
Reviewed-by: Joakim Bech <[email protected]>
Makes creation of shm pools more flexible by adding new more primitive
functions to allocate a shm pool. This makes it easier to add driver
specific shm pool management.

Signed-off-by: Jens Wiklander <[email protected]>
Added new ioctl to allow users register own buffers as a shared memory.

Signed-off-by: Jens Wiklander <[email protected]>
Signed-off-by: Volodymyr Babchuk <[email protected]>
These two function will be needed for shared memory registration in OP-TEE

Signed-off-by: Volodymyr Babchuk <[email protected]>
struct optee_smc_calls_revision_result result;
} res = {
.result = {
.reserved0 = 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're now actually using this member it should have a proper name.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried about breaking backwards compatibility, but since (1) it is in-kernel code and (2) no one is supposed to use a reserved field, it sounds reasonable indeed.


if ((u32)res.result.reserved0)
snprintf(id, sizeof(id), " (%08x)", (u32)res.result.reserved0);
pr_info("revision %lu.%lu%s", res.result.major, res.result.minor, id);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stuff with id looks a bit more complicated than just:

if (res.result.reserved0)
     pr_info("revision %lu.%lu (%08x)", res.result.major, res.result.minor,
             (u32)res.result.reserved0);
else
     pr_info("revision %lu.%lu", res.result.major, res.result.minor);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup will fix

@jforissier
Copy link
Author

Update

&res.smccc);

if (res.result.build_id)
pr_info("revision %lu.%lu (%08x)", res.result.major,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With %08lx we wouldn't need to cast build_id.

@jforissier
Copy link
Author

Comment addressed.

@jenswi-linaro
Copy link

Looks good.

@jforissier
Copy link
Author

OK thanks Jens. I'll post to LKML.

In the OPTEE_SMC_CALL_GET_OS_REVISION request, the previously reserved
parameter a2 is now documented as being an optional build identifier
(such as an SCM revision or commit ID, for instance).

A new structure optee_smc_call_get_os_revision_result is introduced to
be used when querying the secure OS version, instead of re-using the
struct defined for OPTEE_SMC_CALLS_REVISION.

Signed-off-by: Jerome Forissier <[email protected]>
When the driver initializes, report the following information
about the OP-TEE OS:
- major and minor version,
- build identifier (if available).

Signed-off-by: Jerome Forissier <[email protected]>
@jforissier jforissier force-pushed the optee branch 3 times, most recently from 221a1ac to 0fd2deb Compare March 8, 2019 09:16
jenswi-linaro pushed a commit to jenswi-linaro/linux-1 that referenced this pull request May 28, 2025
If, during a mremap() operation for a hugetlb-backed memory mapping,
copy_vma() fails after the source vma has been duplicated and opened (ie. 
vma_link() fails), the error is handled by closing the new vma.  This
updates the hugetlbfs reservation counter of the reservation map which at
this point is referenced by both the source vma and the new copy.  As a
result, once the new vma has been freed and copy_vma() returns, the
reservation counter for the source vma will be incorrect.

This patch addresses this corner case by clearing the hugetlb private page
reservation reference for the new vma and decrementing the reference
before closing the vma, so that vma_close() won't update the reservation
counter.  This is also what copy_vma_and_data() does with the source vma
if copy_vma() succeeds, so a helper function has been added to do the
fixup in both functions.

The issue was reported by a private syzbot instance and can be reproduced
using the C reproducer in [1].  It's also a possible duplicate of public
syzbot report [2].  The WARNING report is:

============================================================
page_counter underflow: -1024 nr_pages=1024
WARNING: CPU: 0 PID: 3287 at mm/page_counter.c:61 page_counter_cancel+0xf6/0x120
Modules linked in:
CPU: 0 UID: 0 PID: 3287 Comm: repro__WARNING_ Not tainted 6.15.0-rc7+ linaro-swg#54 NONE
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-2-gc13ff2cd-prebuilt.qemu.org 04/01/2014
RIP: 0010:page_counter_cancel+0xf6/0x120
Code: ff 5b 41 5e 41 5f 5d c3 cc cc cc cc e8 f3 4f 8f ff c6 05 64 01 27 06 01 48 c7 c7 60 15 f8 85 48 89 de 4c 89 fa e8 2a a7 51 ff <0f> 0b e9 66 ff ff ff 44 89 f9 80 e1 07 38 c1 7c 9d 4c 81
RSP: 0018:ffffc900025df6a0 EFLAGS: 00010246
RAX: 2edfc409ebb44e00 RBX: fffffffffffffc00 RCX: ffff8880155f0000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: dffffc0000000000 R08: ffffffff81c4a23c R09: 1ffff1100330482a
R10: dffffc0000000000 R11: ffffed100330482b R12: 0000000000000000
R13: ffff888058a882c0 R14: ffff888058a882c0 R15: 0000000000000400
FS:  0000000000000000(0000) GS:ffff88808fc53000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004b33e0 CR3: 00000000076d6000 CR4: 00000000000006f0
Call Trace:
 <TASK>
 page_counter_uncharge+0x33/0x80
 hugetlb_cgroup_uncharge_counter+0xcb/0x120
 hugetlb_vm_op_close+0x579/0x960
 ? __pfx_hugetlb_vm_op_close+0x10/0x10
 remove_vma+0x88/0x130
 exit_mmap+0x71e/0xe00
 ? __pfx_exit_mmap+0x10/0x10
 ? __mutex_unlock_slowpath+0x22e/0x7f0
 ? __pfx_exit_aio+0x10/0x10
 ? __up_read+0x256/0x690
 ? uprobe_clear_state+0x274/0x290
 ? mm_update_next_owner+0xa9/0x810
 __mmput+0xc9/0x370
 exit_mm+0x203/0x2f0
 ? __pfx_exit_mm+0x10/0x10
 ? taskstats_exit+0x32b/0xa60
 do_exit+0x921/0x2740
 ? do_raw_spin_lock+0x155/0x3b0
 ? __pfx_do_exit+0x10/0x10
 ? __pfx_do_raw_spin_lock+0x10/0x10
 ? _raw_spin_lock_irq+0xc5/0x100
 do_group_exit+0x20c/0x2c0
 get_signal+0x168c/0x1720
 ? __pfx_get_signal+0x10/0x10
 ? schedule+0x165/0x360
 arch_do_signal_or_restart+0x8e/0x7d0
 ? __pfx_arch_do_signal_or_restart+0x10/0x10
 ? __pfx___se_sys_futex+0x10/0x10
 syscall_exit_to_user_mode+0xb8/0x2c0
 do_syscall_64+0x75/0x120
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x422dcd
Code: Unable to access opcode bytes at 0x422da3.
RSP: 002b:00007ff266cdb208 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: 0000000000000001 RBX: 00007ff266cdbcdc RCX: 0000000000422dcd
RDX: 00000000000f4240 RSI: 0000000000000081 RDI: 00000000004c7bec
RBP: 00007ff266cdb220 R08: 203a6362696c6720 R09: 203a6362696c6720
R10: 0000200000c00000 R11: 0000000000000246 R12: ffffffffffffffd0
R13: 0000000000000002 R14: 00007ffe1cb5f520 R15: 00007ff266cbb000
 </TASK>
============================================================

Link: https://lkml.kernel.org/r/20250523-warning_in_page_counter_cancel-v2-1-b6df1a8cfefd@igalia.com
Link: https://people.igalia.com/rcn/kernel_logs/20250422__WARNING_in_page_counter_cancel__repro.c [1]
Link: https://lore.kernel.org/all/[email protected]/ [2]
Signed-off-by: Ricardo Cañuelo Navarro <[email protected]>
Suggested-by: Lorenzo Stoakes <[email protected]>
Reviewed-by: Liam R. Howlett <[email protected]>
Cc: Florent Revest <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: Vlastimil Babka <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
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

Successfully merging this pull request may close these issues.

8 participants