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.

Sign up for GitHub

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

Jump to bottom

Add missing DMA Buf detach #87

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

Add missing DMA Buf detach #87

wants to merge 29 commits into from

Conversation

jforissier
Copy link

@jforissier jforissier commented Feb 4, 2021 •
edited
Loading 

From: Olivier Masse <[email protected]>

Fix a memory leak in tee_shm_release.

Signed-off-by: Olivier Masse <[email protected]>

jenswi-linaro and others added 29 commits October 12, 2020 10:10
@jenswi-linaro @jforissier
Add upstream-tee-subsys-patches.txt
9e38023 
Adds upstream-tee-subsys-patches.txt describing all upstream patches
related to the TEE subsystem.

Signed-off-by: Jens Wiklander <[email protected]>
[jf: rebase on top of v4.18]
Signed-off-by: Jerome Forissier <[email protected]>
@etienne-lms @jforissier
tee: new ioctl to a register tee_shm from a dmabuf file descriptor
9d5c773 
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]>
[jf: squash fixup commit ("tee: fix unbalanced context refcount in
 register shm from fd")]
[jf: rebase onto v5.9-rc8]
Signed-off-by: Jerome Forissier <[email protected]>
@jbech-linaro @jforissier
arm64: dt: Remove timer from mt8173 **not for mainline**
595188c 
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]>
@jenswi-linaro @jforissier
arm64: dt: OP-TEE for foundation-v8 **not for mainline**
1d5f2c3 
Configures foundation-v8 with OP-TEE.

Signed-off-by: Jens Wiklander <[email protected]>
[jf: rebase onto v5.9-rc7]
Signed-off-by: Jerome Forissier <[email protected]>
@jenswi-linaro @jforissier
arm64: dt: OP-TEE for Juno **not for mainline**
ef79f9e 
Configures Juno with OP-TEE.

Reviewed-by: Pascal Brand <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
[jf: rebase onto v5.9-rc7]
Signed-off-by: Jerome Forissier <[email protected]>
@jenswi-linaro @jforissier
arm64: dt: Add no-map to the reserved-memory node for OP-TEE for foun…
767ffb6 
…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]>
@jenswi-linaro @jforissier
arm64: dt: Add no-map to the reserved-memory node for OP-TEE for juno…
c9108c8 
… **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]>
@jbech-linaro @jforissier
arm64: dt: Add OP-TEE firmware to mt8173 **not for mainline**
f9029fe 
Signed-off-by: Joakim Bech <[email protected]>
Reviewed-by: Pascal Brand <[email protected]>
Reviewed-by: Jerome Forissier <[email protected]>
@jforissier
OP-TEE Benchmark **not for mainline**
9f5731c 
Add Benchmark support

Reviewed-by: Joakim Bech <[email protected]>
Signed-off-by: Igor Opaniuk <[email protected]>
[jf: squash fixup commit "tee: optee: optee_bench.h: remove useless include **not for mainline**"]
[jf: rebase onto v5.9-rc7]
Signed-off-by: Jerome Forissier <[email protected]>
@jforissier
tee: support of allocating DMA shared buffers **not for mainline**
41e9290 
Add support of allocating DMA shared buffers via RPC calls. The main
difference with OPTEE_MSG_RPC_SHM_TYPE_KERNEL is that SHM pool manager for
shared memory exported to user space is explicitly chosen.

As dma-buf is used for exporting buffers to userspace, it provides a
possiblity to mmap an  allocated SHM buffer into multiple TEE client
applications (unlike OPTEE_MSG_RPC_SHM_TYPE_APPL, which leverages
tee-supplicant for private allocations).

Such buffers should be used only for internal purposes, when there
is a need to share meta data between different OP-TEE components (for
debugging/profiling purposes).

Signed-off-by: Igor Opaniuk <[email protected]>
[jf: squash fixup commit]
Signed-off-by: Jerome Forissier <[email protected]>
@jenswi-linaro @jforissier
upstream-tee-subsys-patches.txt: patches up to v4.20-rc1
0b1c103 
Records patches available upstream up to v4.20-rc1.

Acked-by: Joakim Bech <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
@jforissier
upstream-tee-subsys-patches.txt: patches up to v5.0
f795a4f 
Records patches available upstream up to v5.0.

Signed-off-by: Jerome Forissier <[email protected]>
@jforissier
upstream-tee-subsys-patches.txt: patches up to v5.1
578ce3c 
Records patches available upstream up to v5.1.

Signed-off-by: Jerome Forissier <[email protected]>
@etienne-lms @jforissier
staging/ion: add Allwinner ION "secure" heap
fa11127 
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]>
@etienne-lms @jforissier
ion: "unmapped" heap for secure data path
11ed7ae 
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]>
@etienne-lms @jforissier
staging/ion: condition unmapped heap to supported architectures
366e674 
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]>
@etienne-lms @jforissier
staging/ion: ARM64 supports ION_UNMAPPED_HEAP
05790c9 
Signed-off-by: Etienne Carriere <[email protected]>
@etienne-lms @jforissier
staging:ion: add a no-map property to ion dmabuf attachment
6af0afc 
Ion unmapped heap aims at not being mapped. This change prevents
Ion from calling dma-mapping support on dma_buf_attach for buffers
in an unmapped heap.

This change is a bit intrusive in the Ion driver. Maybe there is
another way to deal with the dma-mapping resources used for the
unmapped heap.

Signed-off-by: Etienne Carriere <[email protected]>
[jf: rebase onto v5.9-rc7]
Signed-off-by: Jerome Forissier <[email protected]>
@jforissier
staging: android: ion: do not clear dma_address of unmapped heap
816b967 
Since commit 54ef5b9 (staging: android: ion: Initialize dma_address
of new sg list") (Linux v4.17), the helper function dup_sg_table() called
by ion_dma_buf_attach() does not preserve the dma_address from the
original SG list. It is a problem for the unmapped heap, because
dma_buf_attach() followed by dma_buf_map_attachment() now returns a SG
table with NULL dma_address, which breaks tee_shm_register_fd().

This commit avoids the dma_address reset for the unmapped heap.

Signed-off-by: Jerome Forissier <[email protected]>
Tested-by: Jerome Forissier <[email protected]> (HiKey960, SDP)
Tested-by: Etienne Carriere <[email protected]> (Qemu_v7/v8, SDP)
@jforissier
upstream-tee-subsys-patches.txt: patches up to v5.4-rc7
bfc70e1 
Records patches available upstream up to v5.4-rc7.

Signed-off-by: Jerome Forissier <[email protected]>
@jforissier
upstream-tee-subsys-patches.txt: patches up to v5.5
10ef4b8 
Records patches available upstream up to v5.5.

Signed-off-by: Jerome Forissier <[email protected]>
@cneveux @jforissier
driver: tee: Handle NULL pointer indication from client
185d754 
  TEE Client introduce a new capability "TEE_GEN_CAP_MEMREF_NULL"
  to handle the support of the shared memory buffer with a NULL pointer.

   This capability depends on TEE Capabilities and driver support.
   Driver and TEE exchange capabilities at driver initialization.

Signed-off-by: Michael Whitfield <[email protected]>
Signed-off-by: Cedric Neveux <[email protected]>
Reviewed-by: Jens Wiklander <[email protected]>
Reviewed-by: Joakim Bech <[email protected]>
Tested-by: Joakim Bech <[email protected]> (QEMU)
@igoropaniuk @jforissier
arm64: dts: hi3798cv200-poplar: memreserve for bootloader
4b38a0a 
Reserve memory for bootloader purposes.

Acked-by: Jerome Forissier <[email protected]>
Signed-off-by: Igor Opaniuk <[email protected]>
@igoropaniuk @jforissier
arm64: dts: hi3798cv200-poplar: add optee node
50c5965 
Add optee node, so OP-TEE driver is probed properly.

Acked-by: Jerome Forissier <[email protected]>
Signed-off-by: Igor Opaniuk <[email protected]>
@etienne-lms @jforissier
ARM: dts: stm32: Define OP-TEE resources on stm32mp15 and enable on DK2
bc1e135 
Define OP-TEE firmware node for stm32mp15 based platforms. The node
if disable by default.

Enable the OP-TEE node and define OP-TEE reserved memory for
stm32mp157c-dk2.

Signed-off-by: Etienne Carriere <[email protected]>
[jf: rebase onto v5.9]
Signed-off-by: Jerome Forissier <[email protected]>
@jforissier
upstream-tee-subsys-patches.txt: patches up to v5.9
d297719 
Records patches available upstream up to v5.9.

Signed-off-by: Jerome Forissier <[email protected]>
@b49020 @jforissier
tee: client UUID: Skip REE kernel login method as well
a1f8ac0 
Since the addition of session's client UUID generation via commit [1],
login via REE kernel method was disallowed. So fix that via passing
nill UUID in case of TEE_IOCTL_LOGIN_REE_KERNEL method as well.

Fixes: e33bcba ("tee: add support for session's client UUID generation") [1]
Signed-off-by: Sumit Garg <[email protected]>
Signed-off-by: Jens Wiklander <[email protected]>
@jforissier
checkpatch: add --kconfig-prefix
28676a5 
Kconfig allows to customize the CONFIG_ prefix via the $CONFIG_
environment variable.  Out-of-tree projects may therefore use Kconfig with
a different prefix, or they may use a custom configuration tool which does
not use the CONFIG_ prefix at all.  Such projects may still want to adhere
to the Linux kernel coding style and run checkpatch.pl.

One example is OP-TEE [1] which does not use Kconfig but does have
configuration options prefixed with CFG_.  It also mostly follows the
kernel coding style and therefore being able to use checkpatch is quite
valuable.

To make this possible, add the --kconfig-prefix command line option.

[1] https://github.com/OP-TEE/optee_os

Signed-off-by: Jerome Forissier <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Acked-by: Joe Perches <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Linus Torvalds <[email protected]>
[jf: commit 3e89ad8 from upstream]
Signed-off-by: Jerome Forissier <[email protected]>
@omasse-linaro @jforissier
Add missing DMA Buf detach
2f283bb 
Fix a memory leak in tee_shm_release.

Signed-off-by: Olivier Masse <[email protected]>
@jforissier jforissier mentioned this pull request Feb 4, 2021
Open
@jenswi-linaro
Copy link

jenswi-linaro commented Feb 5, 2021 •
edited
Loading

Might be worth noting that this is not in upstream. (Edit: or am I missing something?)
A fixes tag would be nice too.

@jforissier
Copy link
Author

Might be worth noting that this is not in upstream. (Edit: or am I missing something?)

No, you're right. If I'm not mistaken this relates the the dma_buf/ion stuff for secure data path which is not upstream.

@omasse-linaro
Copy link

I'm not sure to understand the problem with checks failure, cause I do not have access to
https://optee.mooo.com:5000/logs/linaro-swg/linux/87/567785836/2f283bb26fd9b7d14aa7a7cc92ef42ca88e417a0

@jbech-linaro
Copy link

I'm not sure to understand the problem with checks failure,

Hi Olivier, to me it looks like a time-out happened when trying to download/clone the gits. Let me restart the job.

cause I do not have access to

That's strange. It should be accessible to anyone on the internet.

@omasse-linaro
Copy link

cause I do not have access to

That's strange. It should be accessible to anyone on the internet.

ok I see, it's because used port is 5000 which is not allowed by my office internet connection.

jenswi-linaro pushed a commit to jenswi-linaro/linux-1 that referenced this pull request Sep 6, 2021
@borkmann
bpf: Fix tail_call_reachable rejection for interpreter when jit failed
5dd0a6b 
During testing of f263a81 ("bpf: Track subprog poke descriptors correctly
and fix use-after-free") under various failure conditions, for example, when
jit_subprogs() fails and tries to clean up the program to be run under the
interpreter, we ran into the following freeze:

  [...]
  torvalds#127/8 tailcall_bpf2bpf_3:FAIL
  [...]
  [   92.041251] BUG: KASAN: slab-out-of-bounds in ___bpf_prog_run+0x1b9d/0x2e20
  [   92.042408] Read of size 8 at addr ffff88800da67f68 by task test_progs/682
  [   92.043707]
  [   92.044030] CPU: 1 PID: 682 Comm: test_progs Tainted: G   O   5.13.0-53301-ge6c08cb33a30-dirty linaro-swg#87
  [   92.045542] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1 04/01/2014
  [   92.046785] Call Trace:
  [   92.047171]  ? __bpf_prog_run_args64+0xc0/0xc0
  [   92.047773]  ? __bpf_prog_run_args32+0x8b/0xb0
  [   92.048389]  ? __bpf_prog_run_args64+0xc0/0xc0
  [   92.049019]  ? ktime_get+0x117/0x130
  [...] // few hundred [similar] lines more
  [   92.659025]  ? ktime_get+0x117/0x130
  [   92.659845]  ? __bpf_prog_run_args64+0xc0/0xc0
  [   92.660738]  ? __bpf_prog_run_args32+0x8b/0xb0
  [   92.661528]  ? __bpf_prog_run_args64+0xc0/0xc0
  [   92.662378]  ? print_usage_bug+0x50/0x50
  [   92.663221]  ? print_usage_bug+0x50/0x50
  [   92.664077]  ? bpf_ksym_find+0x9c/0xe0
  [   92.664887]  ? ktime_get+0x117/0x130
  [   92.665624]  ? kernel_text_address+0xf5/0x100
  [   92.666529]  ? __kernel_text_address+0xe/0x30
  [   92.667725]  ? unwind_get_return_address+0x2f/0x50
  [   92.668854]  ? ___bpf_prog_run+0x15d4/0x2e20
  [   92.670185]  ? ktime_get+0x117/0x130
  [   92.671130]  ? __bpf_prog_run_args64+0xc0/0xc0
  [   92.672020]  ? __bpf_prog_run_args32+0x8b/0xb0
  [   92.672860]  ? __bpf_prog_run_args64+0xc0/0xc0
  [   92.675159]  ? ktime_get+0x117/0x130
  [   92.677074]  ? lock_is_held_type+0xd5/0x130
  [   92.678662]  ? ___bpf_prog_run+0x15d4/0x2e20
  [   92.680046]  ? ktime_get+0x117/0x130
  [   92.681285]  ? __bpf_prog_run32+0x6b/0x90
  [   92.682601]  ? __bpf_prog_run64+0x90/0x90
  [   92.683636]  ? lock_downgrade+0x370/0x370
  [   92.684647]  ? mark_held_locks+0x44/0x90
  [   92.685652]  ? ktime_get+0x117/0x130
  [   92.686752]  ? lockdep_hardirqs_on+0x79/0x100
  [   92.688004]  ? ktime_get+0x117/0x130
  [   92.688573]  ? __cant_migrate+0x2b/0x80
  [   92.689192]  ? bpf_test_run+0x2f4/0x510
  [   92.689869]  ? bpf_test_timer_continue+0x1c0/0x1c0
  [   92.690856]  ? rcu_read_lock_bh_held+0x90/0x90
  [   92.691506]  ? __kasan_slab_alloc+0x61/0x80
  [   92.692128]  ? eth_type_trans+0x128/0x240
  [   92.692737]  ? __build_skb+0x46/0x50
  [   92.693252]  ? bpf_prog_test_run_skb+0x65e/0xc50
  [   92.693954]  ? bpf_prog_test_run_raw_tp+0x2d0/0x2d0
  [   92.694639]  ? __fget_light+0xa1/0x100
  [   92.695162]  ? bpf_prog_inc+0x23/0x30
  [   92.695685]  ? __sys_bpf+0xb40/0x2c80
  [   92.696324]  ? bpf_link_get_from_fd+0x90/0x90
  [   92.697150]  ? mark_held_locks+0x24/0x90
  [   92.698007]  ? lockdep_hardirqs_on_prepare+0x124/0x220
  [   92.699045]  ? finish_task_switch+0xe6/0x370
  [   92.700072]  ? lockdep_hardirqs_on+0x79/0x100
  [   92.701233]  ? finish_task_switch+0x11d/0x370
  [   92.702264]  ? __switch_to+0x2c0/0x740
  [   92.703148]  ? mark_held_locks+0x24/0x90
  [   92.704155]  ? __x64_sys_bpf+0x45/0x50
  [   92.705146]  ? do_syscall_64+0x35/0x80
  [   92.706953]  ? entry_SYSCALL_64_after_hwframe+0x44/0xae
  [...]

Turns out that the program rejection from e411901 ("bpf: allow for tailcalls
in BPF subprograms for x64 JIT") is buggy since env->prog->aux->tail_call_reachable
is never true. Commit ebf7d1f ("bpf, x64: rework pro/epilogue and tailcall
handling in JIT") added a tracker into check_max_stack_depth() which propagates
the tail_call_reachable condition throughout the subprograms. This info is then
assigned to the subprogram's func[i]->aux->tail_call_reachable. However, in the
case of the rejection check upon JIT failure, env->prog->aux->tail_call_reachable
is used. func[0]->aux->tail_call_reachable which represents the main program's
information did not propagate this to the outer env->prog->aux, though. Add this
propagation into check_max_stack_depth() where it needs to belong so that the
check can be done reliably.

Fixes: ebf7d1f ("bpf, x64: rework pro/epilogue and tailcall handling in JIT")
Fixes: e411901 ("bpf: allow for tailcalls in BPF subprograms for x64 JIT")
Co-developed-by: John Fastabend <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Signed-off-by: John Fastabend <[email protected]>
Signed-off-by: Alexei Starovoitov <[email protected]>
Acked-by: Maciej Fijalkowski <[email protected]>
Link: https://lore.kernel.org/bpf/618c34e3163ad1a36b1e82377576a6081e182f25.1626123173.git.daniel@iogearbox.net
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@jforissier @jenswi-linaro @omasse-linaro @jbech-linaro @vchong @etienne-lms @cneveux @igoropaniuk @b49020