Re: [RFC PATCH v2 0/4] Mark Kprobe debug exception paths noinstr

[Hongyan Xia <hongyan.xia@transsion.com>]

On 11/3/2026 11:42 PM, Ada Couprie Diaz wrote:
> On 30/10/2026 14:23, Will Deacon wrote:
> 
>> [+Ada]
> Thanks for bringing this to my attention !
>> On Thu, Aug 13, 2026 at 06:47:31AM +0000, Hongyan Xia wrote:
>>> From: Hongyan Xia <hongyan.xia@transsion.com>
>>>
>>> Commit 879a6754d3d1 ("arm64: kprobes: Only handle faults originating
>>> from XOL slot") and 23f851ac0078 ("arm64: kprobes: Allow reentering
>>> kprobes while single-stepping") band-aided two corner cases in the arm64
>>> kprobe path: Nested page faults inside Kprobe and nested Kprobe SS
>>> handling. Although the two patches were taken, Will asked whether we
>>> should keep patching things like this each time we find a corner case,
>>> and whether making Kprobe noinstr is the correct answer [1].
>>>
>>> This series attempts to do exactly that: Making the Kprobe path noinstr.
>>> The kprobe handlers become noinstr end to end. Apart from marking
>>> functions noinstr, we also force-inline some static inline helper
>>> functions because certain compilers still outline them, breaking
>>> noinstr.
>>>
>>> Note that arm64 has no objtool so at the moment noinstr is more of a
>>> contract that must be followed, instead of actually enforcing control
>>> flow. Instrumentation can still happen if someone with root privileges
>>> does something stupid, but now it is regarded as a bug rather than a
>>> legitimate use case that the kernel needs to cover.
> 
> Just to make sure I am not missing something : as far as I can tell,
> Kprobe checks the section the instruction to be instrumented is in
> and will refuse to instrument anything within `.noinstr.text`.[0]

Yes, exactly.

> Given that this series converts the whole call trees of the Kprobe
> functions to `noinstr`, should that not make it safe ?
> (My understanding is that Kprobe is the only one checking the noinstr
> section, all the other instrumentations and sanitizers do not.)

This is the sad part. Yes, it refuses to attach to noinstr sections, but 
noinstr sections could just call normal .text functions which can be 
instrumented, so we are still not safe.

This is exactly the point of objtool, statically validating that the 
control flow simply cannot escape .noinstr sections.

>>> [1] https://apc01.safelinks.protection.outlook.com/? 
>>> url=https%3A%2F%2Flore.kernel.org%2Fall%2FalpuL10h7-OK2hFb%40willie- 
>>> the- 
>>> truck%2F&data=05%7C02%7Chongyan.xia%40transsion.com%7C964feb00a6854510ed3008df39c11576%7C2e8503a62d0143338e366ab7c8cd7ae2%7C0%7C0%7C639293173603733128%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=co2FrgXxUfoT3bmrblgDn4G4LHhNGfXGSuuNBGrTYF4%3D&reserved=0
>>>
>>> Changed in v2:
>>> - Remove instrumentation_{begin/end}() markers as they do nothing
>>> - Fold __always_inline into the patches that actually need them
>>> - Remove the debug exception path changes unrelated to Kprobe. This
>>>    series is only about Kprobe for now
>>> - Drop the kprobe_page_fault() patch for now. It's a question whether we
>>>    even need it at all. Will send a patch for it separately.
>>>
>>> Hongyan Xia (4):
>>>    arm64/entry: Make debug_exception_enter/exit() noinstr
>>>    arm64/debug: Make the Kprobe functions noinstr
>>>    arm64/kprobes: Make the entire Kprobe noinstr
>>>    Revert "arm64: kprobes: Allow reentering kprobes while
>>>      single-stepping"
>>>
>>>   arch/arm64/include/asm/esr.h       |  2 +-
>>>   arch/arm64/include/asm/kprobes.h   | 15 ++------
>>>   arch/arm64/include/asm/percpu.h    |  2 +-
>>>   arch/arm64/include/asm/preempt.h   |  4 +-
>>>   arch/arm64/include/asm/ptrace.h    |  4 +-
>>>   arch/arm64/kernel/debug-monitors.c |  6 +--
>>>   arch/arm64/kernel/entry-common.c   | 13 ++-----
>>>   arch/arm64/kernel/probes/kprobes.c | 59 ++++++++++--------------------
>>>   include/linux/kprobes.h            |  8 ++--
>> Given the lack of objtool enforcement (as you note above), I wonder
>> whether it would be more robust to do this on a per-compilation unit
>> basis? i.e. disable instrumentation for debug-monitors.c, entry-common.c
>> and kprobes.c?
> 
> I think that marking whole compilations units as `noinstr` would be 
> great too,

That would be cool, indeed.

> though I am not sure how it would affect some other sanitizers/ 
> instrumentation.
> 
>> Unfortunately, I couldn't find a way to do that in the Makefile :/ In
>> which case, perhaps this series is the best we can do, but I thought I'd
>> ask the question in case somebody has any bright ideas.
>>
>> Will
> 
> I found out the same thing when I initially started working on making sure
> alternative-callbacks where `noinstr`/patch safe, so this pushed me to dig
> a bit further.
> 
> 
> There are two parts to this, the first one I believe to be relatively 
> simple :
> GCOV, KCOV, KASAN, KCSAN, KMSAN, UBSAN and ftrace all have a way to be
> turned off for specific compilation units in the Makefiles. We already
> use them in some places : `KCOV_INSTRUMENT_entry-common.o := n`.
> 
> We could introduce a similar variable to disable all instrumentations,
> re-using what is already in place : something like
> `NO_INSTRUMENT_entry-common.o := 1`, which would be checked before all
> the other control variables in `scripts/Makefile.lib` and override them.
> 
> 
> The second part is Kprobe itself, which would be much more painful to
> disable as it does not rely on compiler instrumentation or other build
> options we could use.
> However, Kprobe /does/ check at runtime if it is allowed to probe
> a location, refusing to do so if it is in `.noinstr.text` or if the 
> function
> name is part of its explicit deny list.
> 
> So, looking at the shenanigans KVM does for the nVHE hypervisor
> symbols[1], I think it could be possible to either :
>   1. move the `.text` section into `.noinstr.text`, or
>   2. add all symbols to the `_kprobe_blacklist` section, as if adding
>      `NOKPROBE_SYMBOL()` to all functions within the compilation unit.
> 
> I played around by hand with the first idea using `objcopy` and was able
> to combine the `.text` section into an existing `.noinstr.text` section,
> preserving both, but that did not update the relocations, so the kernel
> immediately crashed.
> This leads me to think that adding the symbols to the `_kprobe_blacklist`
> section with a LD script might be the more straightforward solution.

Apart from linker wizardry, the whole noinstr machinery should still 
allow small code snippets to be instrumentable, namely 
instrumentation_{begin/end}() regions. If the whole-compile-unit-noinstr 
implementation doesn't allow it, it might face resistance upstream.

> I have no experience with LD scripts but I would be happy to have a go,
> if that sounds like something that could work *and* worth the effort.
> Otherwise I think this series might be the best we can do !
> 
> Kind regards,
> Ada
> 
> [0] : https://apc01.safelinks.protection.outlook.com/? 
> url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fkernel%2Fkprobes.c%3Fid%3D6628e7d4c6b25c8d18f1345e3a20d21829aae7e4%23n2840&data=05%7C02%7Chongyan.xia%40transsion.com%7C964feb00a6854510ed3008df39c11576%7C2e8503a62d0143338e366ab7c8cd7ae2%7C0%7C0%7C639293173603788749%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=kE%2Fy%2FzHhDV42qutiIkFqI4emK4vQ1Y2bWf0rQ%2FYYR44%3D&reserved=0
> [1] : https://apc01.safelinks.protection.outlook.com/? 
> url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Farch%2Farm64%2Fkvm%2Fhyp%2Fnvhe%2FMakefile%3Fid%3D6628e7d4c6b25c8d18f1345e3a20d21829aae7e4%23n38&data=05%7C02%7Chongyan.xia%40transsion.com%7C964feb00a6854510ed3008df39c11576%7C2e8503a62d0143338e366ab7c8cd7ae2%7C0%7C0%7C639293173603834481%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Ocq80oowgwqVrxjfWJWmQZAkk6y%2BWNnY%2B2qgavLhJcQ%3D&reserved=0