linux-kernel - Re: [RFC PATCH 1/2] x86/kprobes: Prohibit kprobing on INT and UD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 28 Jan 2024 15:09:50 -0600
From: Jinghao Jia <jinghao7@...inois.edu>
To: Xin Li <xin@...or.com>, "Masami Hiramatsu (Google)"
 <mhiramat@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>
Cc: linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] x86/kprobes: Prohibit kprobing on INT and UD



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) <mhiramat@...nel.org>
>> Signed-off-by: Jinghao Jia <jinghao7@...inois.edu>
>> ---
>>   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)
> 

Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (841 bytes)