[Deepin-Kernel-SIG] [linux 6.6.y] [HAOC] some HAOC fixes - #2077
[Deepin-Kernel-SIG] [linux 6.6.y] [HAOC] some HAOC fixes#2077Avenger-285714 wants to merge 13 commits into
Conversation
modpost reports a section mismatch: WARNING: modpost: vmlinux: section mismatch in reference: __create_pgd_mapping_for_iee+0x54 (section: .text.unlikely.) -> __create_pgd_mapping_for_iee_locked (section: .init.text) __create_pgd_mapping_for_iee() lacks a __init annotation, so the compiler places it in a non-init section (.text.unlikely) while it calls __create_pgd_mapping_for_iee_locked(), which lives in .init.text. After init memory is freed, that reference would dangle, hence the mismatch warning. The only caller is __map_memblock_for_iee(), itself __init, reached exclusively from iee_init_mappings() during paging_init(). The call can never happen at runtime, so the wrapper belongs in .init.text too. Annotate it __init instead of __ref: the reference is genuinely init-only, and this also lets the wrapper's code be freed by free_initmem(), slightly reducing the runtime memory footprint. Fixes: 76baf5e ("HAOC: Add support for AArch64 Isolated Execution Environment(IEE).") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
The declaration of iee_init_mappings() in asm/haoc/iee.h lacks the __init annotation present on its definition in iee-mmu.c and on the duplicate declaration in asm/haoc/iee-mmu.h. Keep the annotations consistent so the init-only nature of the function is visible at every declaration site. No functional change. Fixes: 76baf5e ("HAOC: Add support for AArch64 Isolated Execution Environment(IEE).") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
The x86 IEE access gate dispatches IEE operations through iee_funcs[], a plain writable .data function pointer table, using a raw indirect call in hand-written assembly: vmlinux.o: warning: objtool: iee_rw_gate+0x2a: indirect call found in RETPOLINE build In a CONFIG_RETPOLINE=y build every indirect branch must go through the retpoline thunks; the gate's "call *(%rax,%rdi,8)" bypasses them, leaving an unprotected indirect branch (Spectre v2/BTI) inside the entry point of a security mechanism. The table makes this worse: under HAOC's own threat model an attacker with arbitrary kernel write (or ROP into the gate with a controlled %rdi) can redirect or index off the writable table and turn the gate into a call-anywhere gadget that runs with CR0.WP cleared. Replace the table with iee_dispatch(), a C function that switches on the op flag and calls each _iee_*() implementation directly. All callees take at most three register arguments and absorb the flag in their first (unused) parameter, so the existing gate ABI is preserved bit-for-bit while the compiler generates retpoline/objtool/IBT compliant code. This also removes the writable dispatch table from the kernel image entirely. An unknown flag used to dereference past the table (a NULL entry or adjacent .data) and jump to a wild pointer. Fail closed instead: panic() unconditionally. BUG() was considered and rejected: with CONFIG_PANIC_ON_OOPS unset and nearly all gate callers in process context, die() would merely kill the current task, leaving the CPU running with CR0.WP cleared, interrupts disabled and the IEE stack leaked -- exactly the state IEE_SIP exists to prevent. Fixes: a8edec3 ("HAOC: Add support for x86 Isolated Execution Environment") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
The hand-written x86 IEE gates are declared as SYM_FUNC but never set up a frame pointer, and they switch to the per-cpu IEE stack in the middle of the function before calling into C code: vmlinux.o: warning: objtool: iee_rwx_gate+0x38: call without frame pointer save/setup With CONFIG_STACK_VALIDATION=y and CONFIG_FRAME_POINTER=y, objtool requires a valid stack frame before any call instruction. The same defect exists in iee_rw_gate and surfaces as soon as its dispatch call becomes a direct one. Establish a real %rbp frame chain in both gates so the frame-pointer unwinder can walk across the stack switch at runtime: the saved %rbp value links the callee frame back to the caller frame regardless of which stack each frame lives on. objtool itself provably cannot model a mid-function switch to a per-cpu stack: once the CFA is %rbp-anchored, "mov %reg, %rsp" degrades it to CFI_UNDEFINED, and the only modelled stack-switch idiom (the "stack swizzle") yields CFI_SP_INDIRECT, which also fails has_valid_stack_frame(). Follow the x86 entry code convention for such code: declare the gates SYM_CODE_* (STT_NOTYPE) to exempt them from function stack validation while keeping the manually maintained frame chain. SYM_CODE_START does not emit ENDBR64. Add it explicitly to both gates; iee_rw_gate is exported, and CONFIG_X86_KERNEL_IBT=y makes objtool flag data relocations (from the __ksymtab entry) to non-ENDBR targets: vmlinux.o: warning: objtool: .export_symbol+0x1348: data relocation to !ENDBR: iee_rw_gate+0x0 Fixes: a8edec3 ("HAOC: Add support for x86 Isolated Execution Environment") Fixes: 7d1832b ("Haoc: Add support for x86 Sensitive Instruction Protection(IEE_SIP)") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
The exit path of iee_rwx_gate unconditionally ORs X86_CR4_SMEP_SMAP into CR4 after running the sensitive-instruction handler: movq %cr4, %rax 1: orq $X86_CR4_SMEP_SMAP, %rax movq %rax, %cr4 andq $X86_CR4_SMEP_SMAP, %rax cmpq $X86_CR4_SMEP_SMAP, %rax jnz 1 With CONFIG_IEE_SIP=y and haoc=1, every native_write_cr4() goes through this gate with no CPU capability check at all. The first call happens early in setup_arch() (init_mem_mapping() -> cr4_set_bits_and_update_boot(X86_CR4_PSE)), so on any CPU without SMAP (or SMEP) the exit write sets an unsupported CR4 bit, which raises #GP(0) on bare metal with interrupts off and kills the boot this early. Under QEMU TCG the unsupported write does not fault but is silently dropped, leaving CR4 in a state the kernel does not expect; the boot then dies slightly later in early_ioremap: [ 0.008249] last_pfn = 0x3ffe0 max_arch_pfn = 0x400000000 [ 0.008743] MTRR map: 4 entries (3 fixed + 1 variable; max 19), built from 8 variable MTRRs [ 0.008956] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WP UC- WT <boot hangs here> QEMU exception log from the failing boot (-cpu qemu64, haoc=1, nokaslr; note CR4 holding neither SMEP nor SMAP): 0: v=0e e=0000 cpl=0 IP=ffffffff81fd4fbe CR2=ffff888000014750 memcpy_orig 1: v=0e e=0000 cpl=0 IP=ffffffff8439a7c9 CR2=ffff888004436ff8 early_ioremap_pmd 2: v=0e e=0000 cpl=0 IP=ffffffff8439a7ea CR2=ffff888003a3bff8 early_ioremap_pmd CR4=00000000000000a0 (PAE|PGE only) The same exit code also misbehaves on SMAP-capable systems: a deliberate clearcpuid=smap is silently overridden, because the gate forces SMAP back on at every exit, leaving hardware CR4 inconsistent with the cpu_tlbstate.cr4 shadow and defeating the administrator's choice. Root cause is twofold: 1. The gate exit forces a constant SMEP|SMAP mask instead of the bits the platform actually enabled. Saving CR4 on entry and restoring it on exit would not be a valid fix either: the gate proxies every CR4 write (the handler applies the requested value with SMEP stripped, as it executes from user-mapped .iee.si_text pages), so restoring the entry value would silently discard all runtime CR4 updates and, since setup_smep()/setup_smap() themselves go through the gate, the two bits could never be set at all. 2. The "read-back verification" loop is dead code: it tests %rax against itself right after the OR, so the comparison always succeeds and the branch is never taken; CR4 is never re-read. There is nothing to retry anyway -- a CR4 write either takes effect or faults. The ENABLE_WP macro carries the same vacuous testq/je pattern for CR0.WP. Introduce iee_cr4_set_mask, the SMEP/SMAP bits the boot CPU genuinely has, initialized in identify_cpu() right before setup_smep()/setup_smap() and after forced capabilities are applied, so clearcpuid= is honoured. The gate exit now re-reads the handler-written CR4 and ORs in only those supported bits, preserving the hardening intent (SMEP/SMAP cannot be cleared through the gate) without ever writing unsupported bits. Early gate users before identify_boot_cpu() (PSE/PGE/PCIDE setup) run with mask == 0, which is correct since neither bit is involved there. Delete the dead retry loops in the gate exit and in ENABLE_WP. Boot-tested with QEMU 10.2.1 (TCG), clang-built kernel: qemu64 and qemu64,+smep with haoc=1, which previously died at the point above, now boot through IEE initialization and the si_test self-test up to the expected missing-rootfs panic; qemu64,+smep,+smap with and without haoc=1 shows no regression; haoc=1 clearcpuid=smap now keeps SMAP cleared (CR4=0x001006f0 at runtime). Fixes: 7d1832b ("Haoc: Add support for x86 Sensitive Instruction Protection(IEE_SIP)") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
With CREDP enabled, every credential operation performed by swapper/0 corrupts init_task. The CREDP token accessors locate a task's struct task_token through __addr_to_iee(tsk): for slab-allocated tasks the IEE alias was remapped to a dedicated token page by iee_alloc_task_token_slab(), but init_task is a kernel image symbol, so its IEE alias (via __kimg_to_iee()) still points at its own physical page, and the token writes the gate performs with CR0.WP cleared land on init_task itself. struct task_token overlays the start of task_struct: token.new_cred (+40) covers task_struct.usage (+40) and task_struct.flags (+44). While swapper forks PID 1/2, _iee_copy_cred() stores the new cred pointer into token.new_cred, i.e. init_task.usage/flags transiently hold the two halves of a direct-mapped pointer until _iee_init_copied_cred() clears them; afterwards init_task.flags is left as 0 (even PF_KTHREAD is lost). The window between the two gate calls is wide (LSM hooks, key management, ...), and if the timer tick lands in it, scheduler_tick() reads that garbage flags word on current. Whenever the randomized direct-map base makes the PF_WQ_WORKER bit (0x20) of the transient value set -- which happens on roughly half of the boots since CONFIG_RANDOMIZE_MEMORY randomizes page_offset_base -- wq_worker_tick() runs on swapper and dereferences kthread_data(&init_task), which is legitimately NULL: [ 0.578469] Oops: 0000 [deepin-community#1] PREEMPT SMP NOPTI [ 0.578469] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G S [ 0.578469] RIP: 0010:wq_worker_tick+0x19/0x180 [ 0.578469] RAX: 0000000000000000 RBX: ffff88a8bec52a40 [ 0.578469] CR2: 0000000000000020 CR4: 00000000001006f0 [ 0.578469] Call Trace: [ 0.578469] <IRQ> [ 0.578469] scheduler_tick+0x13d/0x340 [ 0.578469] update_process_times+0x7b/0x90 [ 0.578469] tick_periodic+0x6b/0x80 [ 0.578469] tick_handle_periodic+0x29/0x90 [ 0.578469] timer_interrupt+0x1d/0x30 [ 0.578469] __common_interrupt+0x49/0xc0 [ 0.578469] common_interrupt+0x92/0xb0 [ 0.578469] </IRQ> [ 0.578469] <TASK> [ 0.578469] asm_common_interrupt+0x2b/0x40 [ 0.578469] RIP: 0010:iee_rw_gate+0x42/0x50 [ 0.578469] ? prepare_creds+0x87/0x300 [ 0.578469] copy_creds+0x85/0x3a0 [ 0.578469] copy_process+0x32a/0x1070 [ 0.578469] Kernel panic - not syncing: Fatal exception in interrupt The arm64 port already handles this with iee_prepare_init_task_token(), which allocates a dedicated token page for init_task and remaps its IEE alias onto it. Port that to x86, reusing the existing iee_set_token_page_valid() helper, and call it from iee_init(): iee_init() runs in mem_init(), which is before swapper's first credential operation in rest_init(). Note that CREDP's token accesses depend on the token-page remapping machinery, which is only built with IEE_PTRP; without it every token write would write through the 1:1 alias. Make that dependency explicit in Kconfig. Fixes: 416542d ("HAOC: Add support for x86 CRED Protection (CREDP).") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
iee_free_slab() returns early when haoc_enabled is set, which is exactly the case its caller runs in: with haoc=1 the deferred slab free is never scheduled, so task_struct and cred slabs are leaked together with their IEE token pages instead of being released. Drop the inverted check, and bail out if the work item allocation fails instead of dereferencing NULL. Fixes: 7806e1c ("HAOC: Support pointer protection for x86 IEE (IEE_PTRP)") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
_iee_stack_init() allocates two pages per CPU: the first one is stored in a local variable, set read-only and then leaked, while the per-cpu stack pointer is derived from the second one, which stays writable. A stack obviously must be writable, so the RO setting on the first page is both misplaced and the page itself is lost. Allocate a single page per CPU, use it as the stack, and drop the bogus set_memory_ro(). Fixes: a8edec3 ("HAOC: Add support for x86 Isolated Execution Environment") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
abort_creds() drops the caller's reference with put_cred() before asking the IEE to clear token.new_cred. put_cred() may be the last reference and release the object, so iee_abort_creds() potentially runs after the cred has been freed. Clear the token first, while the object is still alive, mirroring how commit_creds() performs the IEE operation before dropping references. Fixes: a7369f4 ("HAOC: CREDP: protect commit_creds() from ROP attack.") Assisted-by: Kimi Code:K3 Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
When haoc_enabled is set, native_write_cr4() routes every CR4 write
through iee_write_cr4() -> _iee_si_handler(IEE_WRITE_CR4) before its
own pinning check runs. The handler's CR4 branch only stripped SMEP
and wrote the value straight to hardware -- unlike the sibling
IEE_WRITE_CR0 branch, which faithfully replicates the CR0.WP pinning
enforcement. The gate exit path only restores SMEP/SMAP (via
iee_cr4_set_mask), so a write that cleared UMIP or FSGSBASE, or set
CET, took effect silently and permanently.
This renders CR4 pinning -- x86's defense-in-depth against CR4
tampering after a control-flow hijack -- void on haoc kernels with
zero warning:
- clearing UMIP re-enables userspace SIDT/SGDT/SLDT/SMSW/STR and
effectively defeats KASLR;
- clearing FSGSBASE #UDs the kernel's own ALTERNATIVE-patched
{RD,WR}GSBASE sites;
- setting CET makes the gate's CR0.WP toggle #GP (SDM Vol. 3A
section 2.5), wedging the IEE gate.
The defect was left half-finished from day one: the introducing
commit turned cr4_pinned_mask/cr4_pinned_bits/cr_pinning into
globals and added extern declarations for them in iee-si.h, yet the
handler only ever consumed cr_pinning (in the CR0 branch).
Replicate the pinning enforcement in the handler, with two
deviations forced by the execution environment:
- SMEP is excluded from the check/force set: the handler executes
from user-mapped .iee.si_text with SMEP already cleared by the
gate, so writing CR4 with SMEP=1 would fault on the very next
instruction fetch. The gate exit restores SMEP/SMAP per
iee_cr4_set_mask and is deliberately left untouched -- forcing
pinned bits in the gate would break setup_smep()/setup_smap(),
which legitimately run through the gate before pinning starts.
- The value is corrected *before* writing instead of the native
write-check-rewrite sequence, so a malicious value never reaches
hardware at all; WARN_ONCE() then fires as usual.
The check is gated on the cr_pinning static key, exactly like the
native path, so early-boot CR4 writes (PSE/PGE/FSGSBASE/SMEP/SMAP/
UMIP setup, all before setup_cr_pinning()) are unaffected.
Note that under IEE, CET is pinned to 0 by design: setup_cet()
refuses to enable it because the gate's WP toggling conflicts with
CR4.CET=1 (commit a8edec3: "IEE depends on CR0.wp"). This
change preserves that behavior and only restores enforcement for
the remaining pinned bits.
Verified by booting with haoc=1 under QEMU: IEE shadow mapping,
.iee.text U-page remapping and si_test all succeed.
Fixes: 7d1832b ("Haoc: Add support for x86 Sensitive Instruction Protection(IEE_SIP)")
Assisted-by: Kimi Code:K3
Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
Upstream keeps cr4_pinned_mask "static const". The IEE_SIP commit dropped both qualifiers so the SI handler could reference it from another TU, and added a runtime reassignment in parse_haoc_enabled() -- which stores exactly the value the initializer already carries (the IEE pinned set is identical to the upstream one), leaving the mask in writable .data for no reason. An attacker with an arbitrary kernel write could zero it and defeat CR4 pinning even on the non-haoc path: a hardening regression against the upstream baseline, not merely a missing improvement. Drop the redundant early-param assignment along with the extern declaration it needed in iee-init.c. With no writer left anywhere, the mask can simply be const again: only the "static" stays dropped (the handler needs cross-TU access), and the extern declaration in iee-si.h gains the const qualifier to match. This is strictly stronger than __ro_after_init: the object lives in .rodata, i.e. read-only for the kernel's entire lifetime rather than only after mark_rodata_ro(), and any future write attempt fails at compile time. Fixes: 7d1832b ("Haoc: Add support for x86 Sensitive Instruction Protection(IEE_SIP)") Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
The decompressor is linked separately (eg. ld.lld -pie) from its own
object list and never sees arch/x86/kernel/haoc/iee/ objects. The
IEE_SIP commit nevertheless templated
#ifdef CONFIG_IEE_SIP
if (haoc_enabled)
iee_write_cr3_early(...);
else
write_cr3(...);
#endif
onto three CR3 writes in ident_map_64.c and one in pgtable_64.c,
apparently as part of mechanically covering every raw CR3 write in
the tree.
The references can never be satisfied in the decompressor:
haoc_enabled is defined only in iee-init.c (main kernel) and is
parsed from the "haoc=" early_param in setup_arch(), long after the
decompressor has exited. No IEE state (shadow mappings, IEE stacks,
SIP page setup) exists at decompression time at all, so the branch
is dead by design -- the original commit even chose the plain
"_early" CR3 write for the true arm, which is exactly what the code
now does unconditionally.
A regular -O2 build links today only by an optimizer accident: both
arms lower to the identical "mov %0,%%cr3" asm (the nested
iee_rwx_gate() call inside native_write_cr3() is dead under the
outer else), so clang folds the branch and drops the dead load of
haoc_enabled. Rebuilding the same translation units at -O0 exposes
the latent breakage immediately:
ld.lld: error: undefined hidden symbol: haoc_enabled
>>> referenced by ident_map_64.c
>>> ident_map_64.o:(initialize_identity_maps)
>>> referenced by ident_map_64.c
>>> ident_map_64.o:(set_clr_page_flags)
>>> referenced by pgtable_64.c
>>> pgtable_64.o:(configure_5level_paging)
>>> referenced 3 more times
ld.lld: error: undefined hidden symbol: iee_rwx_gate
>>> referenced by pgtable_64.c
>>> pgtable_64.o:(iee_write_cr3)
>>> referenced by ident_map_64.c
>>> ident_map_64.o:(iee_write_cr3)
Any change that makes the two arms diverge, a different compiler,
or a different optimization level re-exposes it.
Remove the four decompressor sites, and fence the IEE hunks in the
shared headers behind !__DISABLE_EXPORTS so the inline chain
(write_cr3() -> native_write_cr3() -> iee_write_cr3()) can never
reintroduce the references into freestanding translation units.
__DISABLE_EXPORTS is already defined for all of boot/compressed (as
well as real-mode and purgatory, both likewise IEE-free), the main
kernel never defines it, and asm/ibt.h already uses exactly this
idiom to keep kernel-runtime mechanisms out of freestanding builds.
desc.h gets the same guard defensively; nothing in the decompressor
includes it today.
No functional change to the decompressor: with the branch folded
away at -O2, the generated code is identical before and after.
Verified with a full LLVM=1 build and a haoc=1 QEMU boot.
Fixes: 7d1832b ("Haoc: Add support for x86 Sensitive Instruction Protection(IEE_SIP)")
Assisted-by: Kimi Code:K3
Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
CREDP's credential operations locate the per-task task_token through the IEE alias of the task and use token->new_cred as the prepare_creds()/commit_creds() validation channel, but the machinery that gives each task a dedicated, protected token page (token page allocation and IEE shadow PTE remapping) is only built with IEE_PTRP. With CREDP=y and IEE_PTRP=n every token write goes through the 1:1 alias and lands on the task itself, corrupting task_struct usage/flags, and the validation value sits in unprotected memory, which makes the ROP check both harmful and useless. The same Kconfig hole was closed on x86 by "HAOC: IEE: Give init_task a dedicated token page on x86"; apply the same select to arm64. Fixes: a7369f4 ("HAOC: CREDP: protect commit_creds() from ROP attack.") Signed-off-by: WangYuli <wangyl5933@chinaunicom.cn>
Reviewer's GuideThis PR reworks the HAOC/IEE low-level x86 (and some arm64) plumbing to remove writable indirect dispatch tables, tighten CR0/CR4 pinning semantics, fix stack and token handling, and clean up early-boot interactions and HAOC conditionals so the IEE security mechanisms are robust and objtool/IBT‑clean. Sequence diagram for iee_rw_gate direct iee_dispatch IEE operation routingsequenceDiagram
actor KernelCaller
participant iee_rw_gate
participant iee_dispatch
participant _iee_memcpy
participant _iee_set_cred_uid
KernelCaller->>iee_rw_gate: iee_rw_gate(flag, arg1, arg2, arg3)
iee_rw_gate->>iee_rw_gate: DISABLE_WP / switch_to_iee_stack
iee_rw_gate->>iee_dispatch: iee_dispatch(flag, arg1, arg2, arg3)
alt flag == IEE_OP_MEMCPY
iee_dispatch->>_iee_memcpy: _iee_memcpy(flag, dst, src, len)
_iee_memcpy-->>iee_dispatch: return
else flag == IEE_OP_SET_CRED_UID
iee_dispatch->>_iee_set_cred_uid: _iee_set_cred_uid(flag, cred, KUIDT_INIT(uid))
_iee_set_cred_uid-->>iee_dispatch: return
else unknown flag
iee_dispatch->>iee_dispatch: panic("iee_dispatch: unknown flag")
end
iee_dispatch-->>iee_rw_gate: return value
iee_rw_gate->>iee_rw_gate: switch_to_kernel_stack / ENABLE_WP
iee_rw_gate-->>KernelCaller: return
Sequence diagram for iee_rwx_gate, _iee_si_handler and CR4 pinning/restoresequenceDiagram
actor KernelCaller
participant iee_rwx_gate
participant _iee_si_handler
participant CPU_CR4
KernelCaller->>iee_rwx_gate: iee_rwx_gate(...)
iee_rwx_gate->>CPU_CR4: clear SMEP/SMAP bits
iee_rwx_gate->>iee_rwx_gate: DISABLE_WP / optional iee_stack switch
iee_rwx_gate->>_iee_si_handler: _iee_si_handler(IEE_WRITE_CR4, new_val)
_iee_si_handler->>CPU_CR4: mov new_val, %cr4
_iee_si_handler-->>iee_rwx_gate: return
iee_rwx_gate->>iee_rwx_gate: ENABLE_WP
iee_rwx_gate->>CPU_CR4: or iee_cr4_set_mask, %cr4 (re-enable supported SMEP/SMAP)
iee_rwx_gate-->>KernelCaller: return
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In _iee_stack_init() the per‑CPU IEE stack pages are no longer marked read‑only via set_memory_ro(); if these stacks are expected to be non‑writable or have specific permissions, consider restoring or explicitly documenting the intended page protections.
- iee_dispatch() panics on any unknown flag, which will hard-stop the machine; if you expect corrupted or unexpected flags to be possible in error paths, consider using a less fatal mechanism (e.g. BUG() or WARN and early return) to avoid turning such cases into unconditional panics.
- iee_prepare_init_task_token() unconditionally panics on allocation failure for the init_task token page; if low-memory situations are plausible at this stage, consider whether a fallback or deferred init path is needed instead of a hard panic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In _iee_stack_init() the per‑CPU IEE stack pages are no longer marked read‑only via set_memory_ro(); if these stacks are expected to be non‑writable or have specific permissions, consider restoring or explicitly documenting the intended page protections.
- iee_dispatch() panics on any unknown flag, which will hard-stop the machine; if you expect corrupted or unexpected flags to be possible in error paths, consider using a less fatal mechanism (e.g. BUG() or WARN and early return) to avoid turning such cases into unconditional panics.
- iee_prepare_init_task_token() unconditionally panics on allocation failure for the init_task token page; if low-memory situations are plausible at this stage, consider whether a fallback or deferred init path is needed instead of a hard panic.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Note: Because of HAOC's bug, Check defconfig fails is true. Related patches will be splatted to a new PR. |
There was a problem hiding this comment.
Pull request overview
Hardens HAOC/IEE dispatch, stack handling, CR4 controls, token initialization, and credential lifecycle management.
Changes:
- Replaces writable x86 IEE dispatch tables with direct dispatch.
- Improves CR4, stack, slab-freeing, and init-task token handling.
- Tightens configuration, early-boot exports, and arm64 initialization annotations.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| kernel/cred.c | Reorders credential abort handling. |
| arch/x86/kernel/haoc/iee/iee-token.c | Adds dedicated init-task token pages. |
| arch/x86/kernel/haoc/iee/iee-si.c | Enforces pinned CR4 bits. |
| arch/x86/kernel/haoc/iee/iee-init.c | Initializes per-CPU stacks and token. |
| arch/x86/kernel/haoc/iee/iee-gate.S | Refactors gate dispatch and stack frames. |
| arch/x86/kernel/haoc/iee/iee-func.c | Restores deferred slab freeing. |
| arch/x86/kernel/haoc/haoc.c | Adds direct IEE dispatcher. |
| arch/x86/kernel/cpu/common.c | Initializes SMEP/SMAP mask. |
| arch/x86/Kconfig | Makes CREDP select pointer protection. |
| arch/x86/include/asm/special_insns.h | Restricts exports and centralizes CR3 writes. |
| arch/x86/include/asm/haoc/iee-token.h | Declares token preparation API. |
| arch/x86/include/asm/haoc/iee-si.h | Exposes CR4 state declarations. |
| arch/x86/include/asm/haoc/haoc.h | Declares dispatcher API. |
| arch/x86/include/asm/desc.h | Restricts IEE exports. |
| arch/x86/boot/compressed/pgtable_64.c | Simplifies early CR3 writes. |
| arch/x86/boot/compressed/ident_map_64.c | Simplifies identity-map CR3 writes. |
| arch/arm64/kernel/haoc/Kconfig | Makes CREDP select pointer protection. |
| arch/arm64/kernel/haoc/iee/iee-mmu.c | Marks mapping helper initialization-only. |
| arch/arm64/include/asm/haoc/iee.h | Marks mapping API initialization-only. |
Suppressed comments (2)
arch/x86/kernel/haoc/iee/iee-gate.S:95
- This second gate has the same ORC regression:
SYM_CODE_STARTplus no unwind hint gives objtool no CFI, so ORC stack traces terminate here despite the%rbpchain. Add validated ORC metadata for the stack-switch path rather than dropping the function metadata entirely.
SYM_CODE_START(iee_rwx_gate)
ENDBR
arch/x86/kernel/haoc/iee/iee-gate.S:113
- The rwx gate also establishes
%rbpafter saving%r12and RFLAGS, leaving RFLAGS at%rbp + 8instead of the caller return address. The frame-pointer unwinder will follow this malformed record if the handler is sampled or faults. Move frame establishment ahead of the extra saves and adjust the epilogue accordingly.
pushq %rbp
movq %rsp, %rbp
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| iee_cr4_set_mask = (cpu_has(c, X86_FEATURE_SMEP) ? X86_CR4_SMEP : 0) | | ||
| (cpu_has(c, X86_FEATURE_SMAP) ? X86_CR4_SMAP : 0); |
| if (!iee_free_slab_work) | ||
| return; |
| SYM_CODE_START(iee_rw_gate) | ||
| ENDBR |
| pushq %rbp | ||
| movq %rsp, %rbp |
There was a problem hiding this comment.
如果目的是允许函数栈能回溯到 iee_gate 之前,是否应该在函数的开头执行 push rbp; mov rbp, rsp ? @Avenger-285714
| /* | ||
| * Re-enable SMEP/SMAP after running the handler. Only the bits the | ||
| * CPU actually supports may be set (setting an unsupported CR4 bit | ||
| * #GPs); iee_cr4_set_mask is initialized in identify_cpu() before | ||
| * setup_smep()/setup_smap(). No read-back retry: a CR4 write either | ||
| * takes effect or faults, there is nothing to retry. | ||
| */ | ||
| movq %cr4, %rax /* value written by the handler */ | ||
| orq iee_cr4_set_mask(%rip), %rax |
There was a problem hiding this comment.
x86 HAOC 依赖 SMAP/SMEP 特性实现 iee_gate,在离开 iee_gate 时应确保 SMAP/SMEP 均启用来保护 iee 区域的安全性。即:如果当前 CPU 不支持 SMAP/SMEP,则不应该使能 HAOC。所以应该在 setup 阶段检查 CPU 对该特性的支持,不支持则 panic。可以参考 https://gitee.com/OpenCloudOS/OpenCloudOS-Kernel/commit/3b25dbdc86f1071eb5900ac25bbc0bf0c3b1383b
Summary by Sourcery
Harden and refactor the HAOC/IEE gates and dispatch paths, improve CR4 pinning and SMEP/SMAP handling, and fix several HAOC initialization and token/cred management issues on x86 and arm64.
Bug Fixes:
Enhancements: