On 1/27/24 13:47, Xin Li wrote: > On 1/26/2024 8:41 PM, Jinghao Jia wrote: >> Both INTs (INT n, INT1, INT3, INTO) and UDs (UD0, UD1, UD2) serve >> special purposes in the kernel, e.g., INT3 is used by KGDB and UD2 is >> involved in LLVM-KCFI instrumentation. At the same time, attaching >> kprobes on these instructions (particularly UDs) will pollute the stack >> trace dumped in the kernel ring buffer, since the exception is triggered >> in the copy buffer rather than the original location. >> >> Check for INTs and UDs in can_probe and reject any kprobes trying to >> attach to these instructions. >> >> Suggested-by: Masami Hiramatsu (Google) <[email protected]> >> Signed-off-by: Jinghao Jia <[email protected]> >> --- >> arch/x86/kernel/kprobes/core.c | 33 ++++++++++++++++++++++++++------- >> 1 file changed, 26 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c >> index e8babebad7b8..792b38d22126 100644 >> --- a/arch/x86/kernel/kprobes/core.c >> +++ b/arch/x86/kernel/kprobes/core.c >> @@ -252,6 +252,22 @@ unsigned long >> recover_probed_instruction(kprobe_opcode_t *buf, unsigned long add >> return __recover_probed_insn(buf, addr); >> } >> +static inline int is_exception_insn(struct insn *insn) > > s/int/bool >
Oh yes, the return type should be bool. Thanks for pointing out!
--Jinghao
>> +{
>> + if (insn->opcode.bytes[0] == 0x0f) {
>> + /* UD0 / UD1 / UD2 */
>> + return insn->opcode.bytes[1] == 0xff ||
>> + insn->opcode.bytes[1] == 0xb9 ||
>> + insn->opcode.bytes[1] == 0x0b;
>> + } else {
>> + /* INT3 / INT n / INTO / INT1 */
>> + return insn->opcode.bytes[0] == 0xcc ||
>> + insn->opcode.bytes[0] == 0xcd ||
>> + insn->opcode.bytes[0] == 0xce ||
>> + insn->opcode.bytes[0] == 0xf1;
>> + }
>> +}
>> +
>> /* Check if paddr is at an instruction boundary */
>> static int can_probe(unsigned long paddr)
>> {
>> @@ -294,6 +310,16 @@ static int can_probe(unsigned long paddr)
>> #endif
>> addr += insn.length;
>> }
>> + __addr = recover_probed_instruction(buf, addr);
>> + if (!__addr)
>> + return 0;
>> +
>> + if (insn_decode_kernel(&insn, (void *)__addr) < 0)
>> + return 0;
>> +
>> + if (is_exception_insn(&insn))
>> + return 0;
>> +
>> if (IS_ENABLED(CONFIG_CFI_CLANG)) {
>> /*
>> * The compiler generates the following instruction sequence
>> @@ -308,13 +334,6 @@ static int can_probe(unsigned long paddr)
>> * Also, these movl and addl are used for showing expected
>> * type. So those must not be touched.
>> */
>> - __addr = recover_probed_instruction(buf, addr);
>> - if (!__addr)
>> - return 0;
>> -
>> - if (insn_decode_kernel(&insn, (void *)__addr) < 0)
>> - return 0;
>> -
>> if (insn.opcode.value == 0xBA)
>> offset = 12;
>> else if (insn.opcode.value == 0x3)
>
OpenPGP_signature.asc
Description: OpenPGP digital signature
