opensbi.git/lib, branch v1.9

lib: sbi_pmu: fix integer overflow in pmu_ctr_idx_validate

2026-06-28T08:34:17Z

pmu_ctr_idx_validate() checks whether counter indices are in range using cbase + sbi_fls(cmask) < total_ctrs. Both operands are unsigned long, so a crafted cbase close to ULONG_MAX causes the addition to wrap around to a small value that passes the comparison. Once validation is bypassed, sbi_pmu_ctr_cfg_match() with the SKIP_MATCH flag uses the overflowed index directly as an array subscript into phs->active_events[], producing an out-of-bounds read in M-mode. Through the firmware-event code path, the same overflowed index reaches fw_counters_data[] and fw_counters_started, giving an attacker OOB write-zero and OOB bit-set primitives in M-mode memory. Fix pmu_ctr_idx_validate() by checking for unsigned overflow before the comparison, and add a secondary bounds check on cidx_first in the SKIP_MATCH path so that even if validation is somehow bypassed in the future, the array access remains bounded. Signed-off-by: liutong Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260624035049.1753003-1-liutong@iscas.ac.cn Signed-off-by: Anup Patel

lib: sbi: Rework misaligned vector load/store

2026-06-17T06:13:38Z

Fix the following issues with misaligned vector load/store: a. Stack overflow: the mask[VLEN_MAX / 8] variable consumes 8K stack space, given VLEN_MAX=65536, overflowing the default-sized stack. There's no need to fetch the whole mask in one go, instead, make it on-demand. Use a 128-byte mask as local buffer to hold the sliding window of mask. For rvv load, this is allowed -- from the spec: "The destination vector register group for a masked vector instruction cannot overlap the source mask register (v0), unless the destination vector register is being written with a mask value (e.g., compares) or the scalar result of a reduction" We don't need to worry about the mask getting overwritten. b. Maintain the value of vstart upon abort (uptrap) to avoid duplicate work. After fault resolution, the instruction can restart from the faulting vstart. For Fault-Only-First loads, reset vstart to 0, as previously done so, to conform to spec. c. Explicitly set VS dirty in VSSTATUS with SET_VS_DIRTY() if faulting from V=1, and if any vector register, including vstart/vl/vtype, gets changed in the handler. It can add 1 unnecessary op to set VS dirty in M/SSTATUS (not VSSTATUS), where the HW already did, but for code simplicity, do it anyway. The overhead should be negligible. Signed-off-by: Bo Gan Tested-by: Anirudh Srinivasan Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260609060024.706-5-ganboing@gmail.com Signed-off-by: Anup Patel

lib: sbi: Add variable-length unprivilege access functions

2026-06-17T06:11:11Z

sbi_load/store_loop read/write variable-length buffer unprivileged. Both function use the widest aligned 8/4/2/1 byte load/stores in each loop to reduce the total number of iterations. Also switch the scalar/vector misaligned handlers to make use of such functions to simplify code. Miscellaneous: remove the unnecessary [taddr] in inline assembly Signed-off-by: Bo Gan Tested-by: Anirudh Srinivasan Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260609060024.706-4-ganboing@gmail.com Signed-off-by: Anup Patel

lib: sbi: Rework and split sbi_misaligned(_v)_tinst_fixup

2026-06-17T06:07:19Z

The load/store address offset between the uptrap and the orig_trap can be derived by orig_trap->tval - uptrap->tval, thus refactor the function prototype for simplicity. For vector load, sbi_misaligned_v_tinst_fixup is introduced. There's no transformed instruction for vector load/store, so null out tinst if the fault is not a guest-page fault. Signed-off-by: Bo Gan Tested-by: Anirudh Srinivasan Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260609060024.706-3-ganboing@gmail.com Signed-off-by: Anup Patel

lib: sbi: cosmetic changes to reduce indentation

2026-06-17T05:53:05Z

In preparation for subsequent patches. Signed-off-by: Bo Gan Tested-by: Anirudh Srinivasan Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260609060024.706-2-ganboing@gmail.com Signed-off-by: Anup Patel

lib: sbi: dbtr: do not unconditionally access tdata2/tdata3 CSRs

2026-06-17T04:12:57Z

The current SBI DBTR extension implementation accesses tdata2 and tdata3 without first checking whether either register is implemented on the underlying hart. This produces an illegal instruction exception on otherwise spec-compliant cores that legitimately omit one or both registers. Per the RISC-V Debug Specification, Chapter 5 (Sdtrig ISA Extension) and Section 5.7 (Trigger Module Registers): Section 5 (Sdtrig introduction): "If Sdtrig is implemented, the Trigger Module must support at least one trigger. Accessing trigger CSRs that are not used by any of the implemented triggers must result in an illegal instruction exception. M-Mode and Debug Mode accesses to trigger CSRs that are used by any of the implemented triggers must succeed, regardless of the current type of the currently selected trigger." Section 5.7 (Trigger Module Registers): "Attempts to access an unimplemented Trigger Module Register raise an illegal instruction exception." Per-register optionality is also explicit: Section 5.7.3 (Trigger Data 2, at 0x7a2): "Trigger-specific data. It is optional if no implemented triggers use it." Section 5.7.4 (Trigger Data 3, at 0x7a3): "Trigger-specific data. It is optional if no implemented triggers use it." Section 5.7.17 (Trigger Extra (RV32), at 0x7a3), which also applies via textra64 on RV64: "All functionality in this register is optional. Any number of upper bits of mhvalue and svalue may be tied to 0. mhselect and sselect may only support 0 (ignore)." Unconditionally accessing tdata2/tdata3 in the install/update/read/ uninstall paths causes SBI calls to fail with an illegal instruction exception on hardware that does not implement one or both CSRs, even if the supervisor-supplied trigger configuration does not require the missing CSR(s). This patch: 1. Introduces tdata_read_safe() / tdata_write_safe() helpers that wrap csr_read_allowed / csr_write_allowed so that an illegal- instruction trap raised by an unimplemented CSR is caught locally rather than propagated. On the read path, a trapped read yields zero; on the write path, the trap is silently absorbed (writes to an unimplemented CSR are no-ops by definition). Every tdata2/tdata3 read and write in the install/update/read/uninstall paths is converted to these helpers. 2. On the install and update paths, rejects requests that program a non-zero trig_tdata2 or trig_tdata3 into an unimplemented CSR with SBI_ERR_NOT_SUPPORTED, matching the SBI spec wording in sections 19.4 / 19.5: "One of the trigger configuration can't be programmed due to unimplemented optional bits in tdata1, tdata2, or tdata3 CSRs." Implementation status is probed once per call via the tdata_implemented() helper. This only catches the "whole CSR unimplemented" case; tied-off WARL bits inside an otherwise- implemented CSR are not caught here and would require programming the trigger and reading the value back for comparison, which can be addressed separately. 3. Enable tdata3 configuration in the debug trigger install path. References: - RISC-V Debug Specification, Chapter 5 (Sdtrig), sections 5, 5.7, 5.7.3, 5.7.4, 5.7.17. - RISC-V SBI Specification v3.0, Chapter 19 (Debug Triggers Extension), sections 19.4, 19.5. Fixes: 97f234f15c96 ("lib: sbi: Introduce the SBI debug triggers extension support") Suggested-by: Nicholas Piggin Suggested-by: Himanshu Chauhan Signed-off-by: David E. Garcia Porras Reviewed-By: Himanshu Chauhan Link: https://lore.kernel.org/r/20260616170118.3515676-1-david.garcia@aheadcomputing.com Signed-off-by: Anup Patel

lib: sbi: Make per-HART stack size configurable via Kconfig

2026-06-16T13:51:11Z

The per-HART stack size for exception/interrupt handling is currently hardcoded to 8192 bytes in SBI_PLATFORM_DEFAULT_HART_STACK_SIZE. This may not be sufficient for platforms with deeper call stacks (e.g. those enabling additional SBI extensions) or may be wasteful for minimal platforms. Introduce a HART_STACK_SIZE Kconfig option in lib/sbi/Kconfig with a valid range of 8192 to 1048576 bytes and a default of 8192 bytes to preserve existing behavior. The SBI_PLATFORM_DEFAULT_HART_STACK_SIZE macro now resolves to CONFIG_HART_STACK_SIZE, allowing all platforms to benefit from a single configuration knob without any source changes. Signed-off-by: Oriol Catalan Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/VI0P192MB3062735A6194BB6DA72083499E002@VI0P192MB3062.EURP192.PROD.OUTLOOK.COM Signed-off-by: Anup Patel

lib: sbi: Rework load/store emulator instruction decoding

2026-06-16T08:12:47Z

Rehaul instruction decoding to fix the following issues: - We assume the XLEN of previous mode is the same as MXLEN. However, RVC instructions decodes differently in RV32 and RV64, so shouldn't have assumed that. - We assume it's a misaligned fault and the load/store offset is 0, i.e., base address == fault address, but access faults can have non-0 offset (on HW supporting misaligned accesses), so platform specific load/store fault handler gets the wrong base address. - No checking of [63:32] of tinst in RV64, which is explicitly required by Privileged ISA 19.6.3. Must reject tinst with non-0 high 32 bits. Thus, fix all the above. For misaligned load/store fault, the address offset should be 0, but we'll validate that on a DEBUG build. On an optmized build, we kill the use of base address, and use trap address instead (same as before), which lets the compiler optimize out imm parsing and other calculations. I also analyzed the behavior of misaligned fault handler before fix. With the following conditions met, it can trigger data corruption: - HW doesn't transform instruction into tinst. - HW doesn't support misaligned load/store, and OS doesn't enable misaligned delegation, thus OpenSBI handler is in effect - HW supports mixed XLEN, and M mode is running RV64, and the trapping mode (U/VS/VU) is running RV32. - The trapping instruction is c.f{l|s}w(sp). Due to the incorrect insn decoding, the trapping instruction would mistakenly be decoded as c.{l|s}d(sp). With this fix, c.f{l|s}w(sp) in RV32 is now emulated correctly. Validation: The patch is validated to have fixed the issue with test cases running on a modified version of QEMU that exposes misaligned faults [1], and a further modified version that removes tinst transformation [2]. The S-mode OS is a local build of Debian Trixie 6.12 kernel that enables COMPAT (RV32), and the U-mode test application exercises all integer and floating-point load/store (RVIFD64/32+RVC64/32) instructions with all possible imm values. The patch is also tested on real HW (Sifive P550/ESWIN EIC7700), which only supports RV64. On P550, the same test was validated both in U mode and VU mode, where the host runs a 6.12 ESWIN vendor kernel that has some ESWIN SoC device driver patches [3] applied, and the guest runs the exact same Debian Trixie 6.12 kernel mentioned above. [1] https://github.com/ganboing/qemu/tree/ganboing-misalign [2] https://github.com/ganboing/qemu/tree/ganboing-misalign-no-tinst [3] https://github.com/sifiveinc/riscv-linux/tree/rel/kernel-6.12/hifive-premier-p550 Fixes: 7219477f7b40 ("lib: Use MTINST CSR in misaligned load/store emulation") Fixes: b5ae8e8a650d ("lib: Add misaligned load/store trap handling") Fixes: 4c112650bbb0 ("lib: sbi: abstract out insn decoding to unify mem fault handlers") Signed-off-by: Bo Gan Tested-by: Anirudh Srinivasan Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260605113214.242-8-ganboing@gmail.com Signed-off-by: Anup Patel

lib: sbi: Do not override emulator callback for vector load/store

2026-06-16T07:55:27Z

It's wrong to override the emulator callback in sbi_trap_emulate_load/ store. The function must respect the callback function passed in the parameter. Hence, let the misaligned emulator callback decide when to use sbi_misaligned_v_ld/st_emulator. To clean up things, also make the following changes: - Add the `insn` parameter to the callback. The trapping insn has been fetched by the caller already, whether transformed or directly loaded, thus saving the trouble in the callback. Note that you must not rely on the length of the `insn`, as it can be a transformed one from tinst - Also the `tcntx` is added, providing the callback with register values to handle vector insn or other customized insns. - Clarify that the read/write length (rlen/wlen) can be 0, in which case it could be a vector load/store or some customized instruction. The callback is responsible to handle it accordingly. Also fixed issues in the sbi_misaligned_v_ld/st_emulator: a. Redirect the trap when OPENSBI_CC_SUPPORT_VECTOR is not available. b. Ensure the return code is >0 when no faults are redirected. Fixes: c2acc5e5b0d8 ("lib: sbi_misaligned_ldst: Add handling of vector load/store") Signed-off-by: Bo Gan Reviewed-by: Anup Patel Tested-by: Anirudh Srinivasan Link: https://lore.kernel.org/r/20260605113214.242-6-ganboing@gmail.com Signed-off-by: Anup Patel

lib: sbi_trap_v_ldst: Redirect unhandled traps

2026-06-15T04:51:03Z

When SBI is built with a compiler that doesn't support vector, the misaligned vector load/store emulation is not built in, the handlers are just stubs. Currently the stubs just return 0, causing sbi_trap_emulate_load() to return without incrementing mepc, meaning the instruction will just fault again, an infinite loop. Fix the stubs to use sbi_trap_redirect(), which forwards the trap to the previous mode, allowing it to be handled there. Fixes: c2acc5e5 ("lib: sbi_misaligned_ldst: Add handling of vector load/store") Signed-off-by: Michael Ellerman Reviewed-by: Anup Patel Link: https://lore.kernel.org/r/20260530-trap-redirect-v1-1-45d4d333d8c9@kernel.org Signed-off-by: Anup Patel