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

Initial port of DTrace for Morello #2309

Open
wants to merge 47 commits into
base: dev
Choose a base branch
from
Open

Initial port of DTrace for Morello #2309

Changes from 1 commit
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
6690b99
dtrace: Remove an unused typedef
markjdb Nov 23, 2024
4b460d7
dtrace tests: Avoid hard-coding paths to required programs
markjdb Jan 22, 2025
375ded9
dtrace: Fix the definition of pc_t
markjdb Jan 22, 2025
2035a30
libdtrace: Use C99 designated initializers for dt_idops_t
markjdb Jan 22, 2025
06a1c6e
dtrace/arm64: Simplify variable declarations in the invop handler
markjdb Jan 22, 2025
6b0068f
dtrace: Use size_t instead of uintptr_t to represent buffer offsets
markjdb Jan 23, 2025
8a0fe9d
dtrace tests: Fix the ATF config variable name
markjdb Jan 23, 2025
ea90dbb
libdtrace: Generalize handling of data models a bit
markjdb Jan 25, 2025
8ad6b87
libdtrace: Use designators to initialize the opcode array
markjdb Jan 29, 2025
c8ad5e1
sdt: Stop defining probe and provider structures as arrays
markjdb Nov 19, 2024
07a7561
dtrace/arm64: Fix enumeration of FBT return probes
markjdb Jan 29, 2025
6b295f9
linker: Improve handling of ifunc symbols
markjdb Jan 29, 2025
1c82ae6
sdt: Use .chericap to store pointers
markjdb Nov 20, 2024
3ef81df
sdt: Avoid setting subobject bounds when adding tracepoint definitions
markjdb Nov 20, 2024
89938db
sdt: Make sure that tracepoints carry a valid capability for patching
markjdb Nov 26, 2024
3fa774a
dtrace/arm64: Add some support for FBT on Morello
markjdb Jan 22, 2025
09f86aa
dtrace: Remove COMPAT_FREEBSD64 hooks
markjdb Jan 22, 2025
d3d26ed
sys/tools/syscalls: Convert sysent probe parameters to intcap_t
markjdb Jan 22, 2025
30b3636
dtrace: Build systrace_freebsd32 only if COMPAT_FREEBSD32 is configured
markjdb Jan 22, 2025
294cfdb
systrace: Use uintcap_t for probe arguments
markjdb Jan 22, 2025
809583f
systrace: Regenerate
markjdb Jan 22, 2025
ac8492f
sdt: Fix format strings for uintptr_t
markjdb Jan 23, 2025
5eea5d2
kinst: Use a vm_pointer_t to store a trampoine address
markjdb Jan 23, 2025
ad51be4
dtrace: Cast to ptraddr_t instead of uintptr_t where appropriate
markjdb Jan 24, 2025
e0a289f
dtrace: Make fuword* and copy(in|out) wrappers capability aware
markjdb Jan 24, 2025
ff2ccfa
dtrace: Avoid applying subobject bounds in dtrace_ecb_aggregation_cre…
markjdb Jan 24, 2025
3f45c07
libdtrace: Add a new data model for CHERI
markjdb Jan 24, 2025
5fbe282
dtrace: Extend use of the dtrace_uarg_t type
markjdb Jan 25, 2025
4450611
dtrace: Convert more incorrect uses of uintptr_t to size_t
markjdb Jan 25, 2025
d952d94
dtrace: Introduce dtrace_difval_t
markjdb Jan 25, 2025
f629227
dtrace: Return a uint64ptr_t from provider getarg routines
markjdb Jan 25, 2025
3eef650
dtrace tests: Exclude json and usdt tests for now
markjdb Jan 25, 2025
88d7794
ctfconvert: Teach the DWARF parser about intcap encodings
markjdb Jan 25, 2025
014fce9
dtrace: Add a new probe error type for CHERI exceptions
markjdb Jan 26, 2025
aa329c8
libdtrace: Add support for printing capabilities
markjdb Jan 26, 2025
316af71
dtrace: Improve a few MD subroutines
markjdb Jan 26, 2025
5a73f78
libdtrace: Use the right width for CPU registers
markjdb Jan 26, 2025
5f5aa50
dtrace: Add capability load and store DIF instructions
markjdb Jan 27, 2025
0d36ec4
libdtrace: Emit LDC and STC DIF instructions when appropriate
markjdb Jan 27, 2025
850a735
dtrace: Permit storing capability-sized values to the trace buffer
markjdb Jan 29, 2025
ba62ac3
dtrace: Fix handling of dynamic state in purecap kernels
markjdb Jan 29, 2025
27bb3b6
dtrace/arm64: Avoid calling memcpy() in dtrace_getarg()
markjdb Jan 29, 2025
e1917f3
dtrace/arm64: Handle purecap function prologues in kinst
markjdb Jan 29, 2025
e17ac4e
dtrace/arm64: Synthesize patchpoint capabilities
markjdb Jan 29, 2025
976f039
share/mk: Enable WITH_DTRACE by default
markjdb Feb 15, 2023
2d0c820
arm64: Enable KDTRACE_HOOKS in GENERIC-MORELLO
markjdb Feb 15, 2023
3003ee1
dtrace: Add a sysctl to block loading of dtrace.ko in CHERI kernels
markjdb Jan 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions sys/kern/link_elf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1790,8 +1790,17 @@ link_elf_symbol_values1(linker_file_t lf, c_linker_sym_t sym,
return (ENOENT);
symval->name = ef->strtab + es->st_name;
val = (caddr_t)ef->address + es->st_value;
if (ELF_ST_TYPE(es->st_info) == STT_GNU_IFUNC)
while (ELF_ST_TYPE(es->st_info) == STT_GNU_IFUNC) {
c_linker_sym_t sym1;
long off;

val = ((caddr_t (*)(void))val)();
if (link_elf_search_symbol(lf, (ptraddr_t)val, &sym1,
&off) != 0 || off != 0)
return (ENOENT);
es = (const Elf_Sym *)sym1;
val = (caddr_t)ef->address + es->st_value;
}
#ifdef __CHERI_PURE_CAPABILITY__
val = make_capability(es, val);
#endif
Copy link
Member

Choose a reason for hiding this comment

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

Is the problem not just that we should be doing make_capability prior to the IFUNC check, rather than after?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, or I don't understand your suggestion.

Consumers of this interface generally don't want to know about ifuncs. What we do for ifuncs is provide a function symbol that refers to the implementation selected by the resolver. Without this change, we were kind of doing that, but the symbol value would be a sentry capability and the size would be that of the ifunc symbol (which in practice is the size of the resolver function). My goal with this change is to eliminate those differences. I don't see how calling make_capability() first would help with that.

Copy link
Member

Choose a reason for hiding this comment

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

The return value of the resolver is what we want to be giving, because it itself comes from some other non-IFUNC capability that the linker's properly constructed. The problem is that make_capability, for an IFUNC, thinks it's been given a capability that's ef->address plus some constant, but it's not, it's got the actual function pointer. On the flip side, we're calling the resolver via a capability that's ef->address plus some constant, so hasn't had its permissions restricted. IFUNCs are in essence "get a capability to the function as normal, but then call it to get the real capability", but we're failing to implement that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I think I see, but addressing that won't fix my problem: the resolver returns a function pointer, which isn't what callers of link_elf_each_function_nameval() expect. I'd have to, at least, unseal the symbol value.

Copy link
Member

@jrtc27 jrtc27 Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

But that's what it returns for normal STT_FUNC today, too? Why are IFUNCs special in that regard?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main consumer of this interface is linker_file_lookup_set(), which definitely requires capabilities, though I expect that linker sets generally don't contain function symbols. I don't think we can return a ptraddr_t here.

I don't quite follow your comment about calling the ifunc resolver with the "right" capability - from what I can, relocate_file1() passes ef->address as relocbase to elf_reloc_internal(), where it's used to derive the resolver capability, just as we do here. You said earlier that we should perhaps call make_capability() before invoking the resolver, which is makes sense to me, but it doesn't solve my problem.

Copy link
Member

Choose a reason for hiding this comment

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

As in the capability we use for the call should have code capability permissions only (and the right bounds, but I believe we don't bound kernel code capabilities even on Morello?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be clear, you are asking me to make that change in this PR?

Copy link
Member

Choose a reason for hiding this comment

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

No, other than that if we're thinking carefully about and rewriting this code to be less weird it might be something that gets folded into the rewrite rather than preserving the oddity.

Copy link
Member

Choose a reason for hiding this comment

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

(I need to go and understand exactly what these interfaces are being used for to be able to assess what the right thing to do about your issue is)

Expand Down Expand Up @@ -1827,8 +1836,17 @@ link_elf_debug_symbol_values(linker_file_t lf, c_linker_sym_t sym,
if (es >= ef->ddbsymtab && es < (ef->ddbsymtab + ef->ddbsymcnt)) {
symval->name = ef->ddbstrtab + es->st_name;
val = (caddr_t)ef->address + es->st_value;
if (ELF_ST_TYPE(es->st_info) == STT_GNU_IFUNC)
while (ELF_ST_TYPE(es->st_info) == STT_GNU_IFUNC) {
c_linker_sym_t sym1;
long off;

val = ((caddr_t (*)(void))val)();
if (link_elf_search_symbol(lf, (ptraddr_t)val, &sym1,
&off) != 0 || off != 0)
return (ENOENT);
es = (const Elf_Sym *)sym1;
val = (caddr_t)ef->address + es->st_value;
}
#ifdef __CHERI_PURE_CAPABILITY__
val = make_capability(es, val);
#endif
Expand Down