linux-kernel - Re: [PATCH 2/3] x86/kprobes: Fix __recover_optprobed

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 16 Feb 2023 10:53:48 +0800
From:   Yang Jihong <yangjihong1@...wei.com>
To:     "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
CC:     <tglx@...utronix.de>, <mingo@...hat.com>, <bp@...en8.de>,
        <dave.hansen@...ux.intel.com>, <x86@...nel.org>, <hpa@...or.com>,
        <naveen.n.rao@...ux.ibm.com>, <anil.s.keshavamurthy@...el.com>,
        <davem@...emloft.net>, <ast@...nel.org>, <peterz@...radead.org>,
        <linux-kernel@...r.kernel.org>,
        <linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] x86/kprobes: Fix __recover_optprobed_insn check
 optimizing logic

Hello Masami,

On 2023/2/15 23:08, Masami Hiramatsu (Google) wrote:
> On Wed, 15 Feb 2023 19:54:29 +0800
> Yang Jihong <yangjihong1@...wei.com> wrote:
> 
>> Since the following commit:
>>
>>    commit f66c0447cca1 ("kprobes: Set unoptimized flag after unoptimizing code")
>>
>> modified the update timing of the KPROBE_FLAG_OPTIMIZED, a optimized_kprobe
>> may be in the optimizing or unoptimizing state when op.kp->flags
>> has KPROBE_FLAG_OPTIMIZED and op->list is not empty.
>>
>> The __recover_optprobed_insn check logic is incorrect, a kprobe in the
>> unoptimizing state may be incorrectly determined as unoptimizing.
>> As a result, incorrect instructions are copied.
> 
> Ah, good catch!
> 
>>
>> The optprobe_queued_unopt function needs to be exported for invoking in
>> arch directory.
>>
>> Fixes: f66c0447cca1 ("kprobes: Set unoptimized flag after unoptimizing code")
> 
> Cc: stable@...r.kernel.org

OK, will add in next version.

> 
>> Signed-off-by: Yang Jihong <yangjihong1@...wei.com>
>> ---
>>   arch/x86/kernel/kprobes/opt.c | 4 ++--
>>   include/linux/kprobes.h       | 1 +
>>   kernel/kprobes.c              | 2 +-
>>   3 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
>> index e57e07b0edb6..3718d6863555 100644
>> --- a/arch/x86/kernel/kprobes/opt.c
>> +++ b/arch/x86/kernel/kprobes/opt.c
>> @@ -46,8 +46,8 @@ unsigned long __recover_optprobed_insn(kprobe_opcode_t *buf, unsigned long addr)
>>   		/* This function only handles jump-optimized kprobe */
>>   		if (kp && kprobe_optimized(kp)) {
>>   			op = container_of(kp, struct optimized_kprobe, kp);
>> -			/* If op->list is not empty, op is under optimizing */
>> -			if (list_empty(&op->list))
>> +			/* If op is [un]optimized or under unoptimizing */
> 
> Hmm, this is a bit confusing comment. If it is unoptimized, the kprobe_optimized() returns false.
> Thus the comment should be /* If op is optimized or under unoptimizing */.
> 

OK, will fix in next version.

Thanks,
Yang.