[3/3] x86/kprobes: Fix arch_check_optimized_kprobe check within optimized_kprobe range
Message ID | 20230215115430.236046-4-yangjihong1@huawei.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp151850wrn; Wed, 15 Feb 2023 04:06:36 -0800 (PST) X-Google-Smtp-Source: AK7set/JLKSSC7ZNTC1wl/8dTLq2rWPoTmJcOaGU7s2seyKxKEM9hFBFF8ve0juZx6mWjfFC5Ks3 X-Received: by 2002:a17:902:9887:b0:19a:8e52:ce0 with SMTP id s7-20020a170902988700b0019a8e520ce0mr1696055plp.58.1676462796533; Wed, 15 Feb 2023 04:06:36 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1676462796; cv=none; d=google.com; s=arc-20160816; b=hcbntLBK/qAZja1nkJPjRbVH1k4fDR1qjoKiBnwXxf6IpmOULATODE4lljNo7DoWhs Vg9i2H3cQ1A4JpZvloz+yv8O1tyGNOcmQcmFSVHjE0eSWrwSFL7vTKv7d9w7J44uDWsI IOq/S0YOsajfod38lpiuKpyl/prAn6g4l4AuVHJzxE1UT9A4ZdQVoo4Gx3Q46e6hMKKz gARknF8jiaE07FDNRA/TyyC46pgfckJcDNYfNUCp8A8Tp2rLpWM+XzaKVYyR+HDXrbnz CRvXryrwBCujS5dJwvrVytrBxDUpIXAaaQbET0ivWIWIZOCk2mpnNr2XppQohIafD3/j hCSg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=SClK/zkhn744LBw1PMZknG61wsNQP8BWPwOdibhCp20=; b=YvsMbqpPyvDcHFo6kVuBe5Z6LEnlI/k1mEfjWq0TR6k6FLfJb2kOd6euS79ormML+z 8CrC94MQa8ay3Y4WvR+OmoyNQ5FfLRS3/HOe+7a7PYbx0Wo3OLZRnE9KJbSLiZbE3zm+ vEeOW7VGKo1mqd3jyrmHNHqxuQUBAVxz+UBatM4WTXWKk8La1q3RKP+HcGzXZbXu8lo8 nT+fKXFJ6HXbNaPseu+RBPLySq1QLY/I2enkGiDgWH9O6nFj40sSTaBOaEo0LKqle9EW XyF371/L2yyKKEoMDrqnLEPXwPeeK/QpE8bkA7uoRXZIxSIpuhdfwaH4bjA1hoz74zCA bCHA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e14-20020a170902cf4e00b0018e06732699si12634751plg.237.2023.02.15.04.06.23; Wed, 15 Feb 2023 04:06:36 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233850AbjBOL4t (ORCPT <rfc822;tebrre53rla2o@gmail.com> + 99 others); Wed, 15 Feb 2023 06:56:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53110 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230147AbjBOL4q (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 15 Feb 2023 06:56:46 -0500 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 79583A264; Wed, 15 Feb 2023 03:56:44 -0800 (PST) Received: from kwepemm600003.china.huawei.com (unknown [172.30.72.54]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4PGxM16qywz16NZb; Wed, 15 Feb 2023 19:54:21 +0800 (CST) Received: from ubuntu1804.huawei.com (10.67.174.61) by kwepemm600003.china.huawei.com (7.193.23.202) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.17; Wed, 15 Feb 2023 19:56:41 +0800 From: Yang Jihong <yangjihong1@huawei.com> To: <tglx@linutronix.de>, <mingo@redhat.com>, <bp@alien8.de>, <dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>, <naveen.n.rao@linux.ibm.com>, <anil.s.keshavamurthy@intel.com>, <davem@davemloft.net>, <mhiramat@kernel.org>, <ast@kernel.org>, <peterz@infradead.org>, <linux-kernel@vger.kernel.org>, <linux-trace-kernel@vger.kernel.org> CC: <yangjihong1@huawei.com> Subject: [PATCH 3/3] x86/kprobes: Fix arch_check_optimized_kprobe check within optimized_kprobe range Date: Wed, 15 Feb 2023 19:54:30 +0800 Message-ID: <20230215115430.236046-4-yangjihong1@huawei.com> X-Mailer: git-send-email 2.30.GIT In-Reply-To: <20230215115430.236046-1-yangjihong1@huawei.com> References: <20230215115430.236046-1-yangjihong1@huawei.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7BIT Content-Type: text/plain; charset=US-ASCII X-Originating-IP: [10.67.174.61] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemm600003.china.huawei.com (7.193.23.202) X-CFilter-Loop: Reflected X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1757898653604062103?= X-GMAIL-MSGID: =?utf-8?q?1757898653604062103?= |
Series |
kprobes: Fix issues related to optkprobe
|
|
Commit Message
Yang Jihong
Feb. 15, 2023, 11:54 a.m. UTC
When arch_prepare_optimized_kprobe calculating jump destination address,
it copies original instructions from jmp-optimized kprobe (see
__recover_optprobed_insn), and calculated based on length of original
instruction.
arch_check_optimized_kprobe does not check KPROBE_FLAG_OPTIMATED when
checking whether jmp-optimized kprobe exists.
As a result, setup_detour_execution may jump to a range that has been
overwritten by jump destination address, resulting in an inval opcode error.
For example, assume that register two kprobes whose addresses are
<func+9> and <func+11> in "func" function.
The original code of "func" function is as follows:
0xffffffff816cb5e9 <+9>: push %r12
0xffffffff816cb5eb <+11>: xor %r12d,%r12d
0xffffffff816cb5ee <+14>: test %rdi,%rdi
0xffffffff816cb5f1 <+17>: setne %r12b
0xffffffff816cb5f5 <+21>: push %rbp
1.Register the kprobe for <func+11>, assume that is kp1, corresponding optimized_kprobe is op1.
After the optimization, "func" code changes to:
0xffffffff816cc079 <+9>: push %r12
0xffffffff816cc07b <+11>: jmp 0xffffffffa0210000
0xffffffff816cc080 <+16>: incl 0xf(%rcx)
0xffffffff816cc083 <+19>: xchg %eax,%ebp
0xffffffff816cc084 <+20>: (bad)
0xffffffff816cc085 <+21>: push %rbp
Now op1->flags == KPROBE_FLAG_OPTIMATED;
2. Register the kprobe for <func+9>, assume that is kp2, corresponding optimized_kprobe is op2.
register_kprobe(kp2)
register_aggr_kprobe
alloc_aggr_kprobe
__prepare_optimized_kprobe
arch_prepare_optimized_kprobe
__recover_optprobed_insn // copy original bytes from kp1->optinsn.copied_insn,
// jump address = <func+14>
3. disable kp1:
disable_kprobe(kp1)
__disable_kprobe
...
if (p == orig_p || aggr_kprobe_disabled(orig_p)) {
ret = disarm_kprobe(orig_p, true) // add op1 in unoptimizing_list, not unoptimized
orig_p->flags |= KPROBE_FLAG_DISABLED; // op1->flags == KPROBE_FLAG_OPTIMATED | KPROBE_FLAG_DISABLED
...
4. unregister kp2
__unregister_kprobe_top
...
if (!kprobe_disabled(ap) && !kprobes_all_disarmed) {
optimize_kprobe(op)
...
if (arch_check_optimized_kprobe(op) < 0) // because op1 has KPROBE_FLAG_DISABLED, here not return
return;
p->kp.flags |= KPROBE_FLAG_OPTIMIZED; // now op2 has KPROBE_FLAG_OPTIMIZED
}
"func" code now is:
0xffffffff816cc079 <+9>: int3
0xffffffff816cc07a <+10>: push %rsp
0xffffffff816cc07b <+11>: jmp 0xffffffffa0210000
0xffffffff816cc080 <+16>: incl 0xf(%rcx)
0xffffffff816cc083 <+19>: xchg %eax,%ebp
0xffffffff816cc084 <+20>: (bad)
0xffffffff816cc085 <+21>: push %rbp
5. if call "func", int3 handler call setup_detour_execution:
if (p->flags & KPROBE_FLAG_OPTIMIZED) {
...
regs->ip = (unsigned long)op->optinsn.insn + TMPL_END_IDX;
...
}
The code for the destination address is
0xffffffffa021072c: push %r12
0xffffffffa021072e: xor %r12d,%r12d
0xffffffffa0210731: jmp 0xffffffff816cb5ee <func+14>
However, <func+14> is not a valid start instruction address. As a result, an error occurs.
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
arch/x86/kernel/kprobes/opt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Wed, 15 Feb 2023 19:54:30 +0800 Yang Jihong <yangjihong1@huawei.com> wrote: > When arch_prepare_optimized_kprobe calculating jump destination address, > it copies original instructions from jmp-optimized kprobe (see > __recover_optprobed_insn), and calculated based on length of original > instruction. > > arch_check_optimized_kprobe does not check KPROBE_FLAG_OPTIMATED when > checking whether jmp-optimized kprobe exists. > As a result, setup_detour_execution may jump to a range that has been > overwritten by jump destination address, resulting in an inval opcode error. OK, good catch !! I missed "delayed unoptimization" case here too. > > For example, assume that register two kprobes whose addresses are > <func+9> and <func+11> in "func" function. > The original code of "func" function is as follows: > > 0xffffffff816cb5e9 <+9>: push %r12 > 0xffffffff816cb5eb <+11>: xor %r12d,%r12d > 0xffffffff816cb5ee <+14>: test %rdi,%rdi > 0xffffffff816cb5f1 <+17>: setne %r12b > 0xffffffff816cb5f5 <+21>: push %rbp > > 1.Register the kprobe for <func+11>, assume that is kp1, corresponding optimized_kprobe is op1. > After the optimization, "func" code changes to: > > 0xffffffff816cc079 <+9>: push %r12 > 0xffffffff816cc07b <+11>: jmp 0xffffffffa0210000 > 0xffffffff816cc080 <+16>: incl 0xf(%rcx) > 0xffffffff816cc083 <+19>: xchg %eax,%ebp > 0xffffffff816cc084 <+20>: (bad) > 0xffffffff816cc085 <+21>: push %rbp > > Now op1->flags == KPROBE_FLAG_OPTIMATED; > > 2. Register the kprobe for <func+9>, assume that is kp2, corresponding optimized_kprobe is op2. > > register_kprobe(kp2) > register_aggr_kprobe > alloc_aggr_kprobe > __prepare_optimized_kprobe > arch_prepare_optimized_kprobe > __recover_optprobed_insn // copy original bytes from kp1->optinsn.copied_insn, > // jump address = <func+14> > > 3. disable kp1: > > disable_kprobe(kp1) > __disable_kprobe > ... > if (p == orig_p || aggr_kprobe_disabled(orig_p)) { > ret = disarm_kprobe(orig_p, true) // add op1 in unoptimizing_list, not unoptimized > orig_p->flags |= KPROBE_FLAG_DISABLED; // op1->flags == KPROBE_FLAG_OPTIMATED | KPROBE_FLAG_DISABLED > ... > > 4. unregister kp2 > __unregister_kprobe_top > ... > if (!kprobe_disabled(ap) && !kprobes_all_disarmed) { > optimize_kprobe(op) > ... > if (arch_check_optimized_kprobe(op) < 0) // because op1 has KPROBE_FLAG_DISABLED, here not return > return; > p->kp.flags |= KPROBE_FLAG_OPTIMIZED; // now op2 has KPROBE_FLAG_OPTIMIZED > } > > "func" code now is: > > 0xffffffff816cc079 <+9>: int3 > 0xffffffff816cc07a <+10>: push %rsp > 0xffffffff816cc07b <+11>: jmp 0xffffffffa0210000 > 0xffffffff816cc080 <+16>: incl 0xf(%rcx) > 0xffffffff816cc083 <+19>: xchg %eax,%ebp > 0xffffffff816cc084 <+20>: (bad) > 0xffffffff816cc085 <+21>: push %rbp > > 5. if call "func", int3 handler call setup_detour_execution: > > if (p->flags & KPROBE_FLAG_OPTIMIZED) { > ... > regs->ip = (unsigned long)op->optinsn.insn + TMPL_END_IDX; > ... > } > > The code for the destination address is > > 0xffffffffa021072c: push %r12 > 0xffffffffa021072e: xor %r12d,%r12d > 0xffffffffa0210731: jmp 0xffffffff816cb5ee <func+14> > > However, <func+14> is not a valid start instruction address. As a result, an error occurs. OK, it has been introduced by the same commit as previous one. (delayed unoptimization) > > Signed-off-by: Yang Jihong <yangjihong1@huawei.com> > --- > arch/x86/kernel/kprobes/opt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c > index 3718d6863555..e6d9bd038401 100644 > --- a/arch/x86/kernel/kprobes/opt.c > +++ b/arch/x86/kernel/kprobes/opt.c > @@ -353,7 +353,7 @@ int arch_check_optimized_kprobe(struct optimized_kprobe *op) > > for (i = 1; i < op->optinsn.size; i++) { > p = get_kprobe(op->kp.addr + i); > - if (p && !kprobe_disabled(p)) > + if (p && (!kprobe_disabled(p) || kprobe_optimized(p))) Hmm, can you rewrite this with kprobe_disarmed() instead of kprobe_disabled()? Since this is checking there are any other kprobes are "armed" on the address where it will be replaced by jump. So it is natural to use "disarmed" check. Thank you, > return -EEXIST; > } > > -- > 2.30.GIT >
Hello Masami, On 2023/2/15 23:48, Masami Hiramatsu (Google) wrote: > On Wed, 15 Feb 2023 19:54:30 +0800 > Yang Jihong <yangjihong1@huawei.com> wrote: > >> When arch_prepare_optimized_kprobe calculating jump destination address, >> it copies original instructions from jmp-optimized kprobe (see >> __recover_optprobed_insn), and calculated based on length of original >> instruction. >> >> arch_check_optimized_kprobe does not check KPROBE_FLAG_OPTIMATED when >> checking whether jmp-optimized kprobe exists. >> As a result, setup_detour_execution may jump to a range that has been >> overwritten by jump destination address, resulting in an inval opcode error. > > OK, good catch !! I missed "delayed unoptimization" case here too. > >> >> For example, assume that register two kprobes whose addresses are >> <func+9> and <func+11> in "func" function. >> The original code of "func" function is as follows: >> >> 0xffffffff816cb5e9 <+9>: push %r12 >> 0xffffffff816cb5eb <+11>: xor %r12d,%r12d >> 0xffffffff816cb5ee <+14>: test %rdi,%rdi >> 0xffffffff816cb5f1 <+17>: setne %r12b >> 0xffffffff816cb5f5 <+21>: push %rbp >> >> 1.Register the kprobe for <func+11>, assume that is kp1, corresponding optimized_kprobe is op1. >> After the optimization, "func" code changes to: >> >> 0xffffffff816cc079 <+9>: push %r12 >> 0xffffffff816cc07b <+11>: jmp 0xffffffffa0210000 >> 0xffffffff816cc080 <+16>: incl 0xf(%rcx) >> 0xffffffff816cc083 <+19>: xchg %eax,%ebp >> 0xffffffff816cc084 <+20>: (bad) >> 0xffffffff816cc085 <+21>: push %rbp >> >> Now op1->flags == KPROBE_FLAG_OPTIMATED; >> >> 2. Register the kprobe for <func+9>, assume that is kp2, corresponding optimized_kprobe is op2. >> >> register_kprobe(kp2) >> register_aggr_kprobe >> alloc_aggr_kprobe >> __prepare_optimized_kprobe >> arch_prepare_optimized_kprobe >> __recover_optprobed_insn // copy original bytes from kp1->optinsn.copied_insn, >> // jump address = <func+14> >> >> 3. disable kp1: >> >> disable_kprobe(kp1) >> __disable_kprobe >> ... >> if (p == orig_p || aggr_kprobe_disabled(orig_p)) { >> ret = disarm_kprobe(orig_p, true) // add op1 in unoptimizing_list, not unoptimized >> orig_p->flags |= KPROBE_FLAG_DISABLED; // op1->flags == KPROBE_FLAG_OPTIMATED | KPROBE_FLAG_DISABLED >> ... >> >> 4. unregister kp2 >> __unregister_kprobe_top >> ... >> if (!kprobe_disabled(ap) && !kprobes_all_disarmed) { >> optimize_kprobe(op) >> ... >> if (arch_check_optimized_kprobe(op) < 0) // because op1 has KPROBE_FLAG_DISABLED, here not return >> return; >> p->kp.flags |= KPROBE_FLAG_OPTIMIZED; // now op2 has KPROBE_FLAG_OPTIMIZED >> } >> >> "func" code now is: >> >> 0xffffffff816cc079 <+9>: int3 >> 0xffffffff816cc07a <+10>: push %rsp >> 0xffffffff816cc07b <+11>: jmp 0xffffffffa0210000 >> 0xffffffff816cc080 <+16>: incl 0xf(%rcx) >> 0xffffffff816cc083 <+19>: xchg %eax,%ebp >> 0xffffffff816cc084 <+20>: (bad) >> 0xffffffff816cc085 <+21>: push %rbp >> >> 5. if call "func", int3 handler call setup_detour_execution: >> >> if (p->flags & KPROBE_FLAG_OPTIMIZED) { >> ... >> regs->ip = (unsigned long)op->optinsn.insn + TMPL_END_IDX; >> ... >> } >> >> The code for the destination address is >> >> 0xffffffffa021072c: push %r12 >> 0xffffffffa021072e: xor %r12d,%r12d >> 0xffffffffa0210731: jmp 0xffffffff816cb5ee <func+14> >> >> However, <func+14> is not a valid start instruction address. As a result, an error occurs. > > OK, it has been introduced by the same commit as previous one. (delayed unoptimization) > OK, will add "Fixes: f66c0447cca1 ("kprobes: Set unoptimized flag after unoptimizing code")" in next version In addition, " Cc: stable@vger.kernel.org" is required, same as the previous patch. >> >> Signed-off-by: Yang Jihong <yangjihong1@huawei.com> >> --- >> arch/x86/kernel/kprobes/opt.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c >> index 3718d6863555..e6d9bd038401 100644 >> --- a/arch/x86/kernel/kprobes/opt.c >> +++ b/arch/x86/kernel/kprobes/opt.c >> @@ -353,7 +353,7 @@ int arch_check_optimized_kprobe(struct optimized_kprobe *op) >> >> for (i = 1; i < op->optinsn.size; i++) { >> p = get_kprobe(op->kp.addr + i); >> - if (p && !kprobe_disabled(p)) >> + if (p && (!kprobe_disabled(p) || kprobe_optimized(p))) > > Hmm, can you rewrite this with kprobe_disarmed() instead of kprobe_disabled()? > Since this is checking there are any other kprobes are "armed" on the address > where it will be replaced by jump. So it is natural to use "disarmed" check. > Yes, It is better to change it to use "kprobe_disarmed", will modify in next version. Thanks, Yang
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c index 3718d6863555..e6d9bd038401 100644 --- a/arch/x86/kernel/kprobes/opt.c +++ b/arch/x86/kernel/kprobes/opt.c @@ -353,7 +353,7 @@ int arch_check_optimized_kprobe(struct optimized_kprobe *op) for (i = 1; i < op->optinsn.size; i++) { p = get_kprobe(op->kp.addr + i); - if (p && !kprobe_disabled(p)) + if (p && (!kprobe_disabled(p) || kprobe_optimized(p))) return -EEXIST; }