Messages in this thread Patch in this message | | | Date | Thu, 9 Feb 2023 23:24:12 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH v2] x86/kprobes: Fix 1 byte conditional jump target |
| |
On Wed, 8 Feb 2023 07:17:08 +0000 Nadav Amit <nadav.amit@gmail.com> wrote:
> From: Nadav Amit <namit@vmware.com> > > Commit 3bc753c06dd0 ("kbuild: treat char as always unsigned") broke > kprobes. Setting a probe-point on 1 byte conditional jump can cause the > kernel to crash, as the branch target is not sign-extended and instead > zero-extended. > > In fact, there is no need for any casting of immediate.value since sign > extension is already done during its decoding in insn_get_immediate(). > > Fix by removing the casting of the 1 byte conditional jump target. > Future patches can also remove the casting (and sign extension) in other > cases in which immediate.value is being used. > > Fixes: 3bc753c06dd0 ("kbuild: treat char as always unsigned") > Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > Suggested-by: Dave Hansen <dave.hansen@intel.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Signed-off-by: Nadav Amit <namit@vmware.com>
Thanks, looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > --- > > v1->v2: Removing the casting completely [Dave] > --- > arch/x86/kernel/kprobes/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c > index b36f3c367cb2..695873c0f50b 100644 > --- a/arch/x86/kernel/kprobes/core.c > +++ b/arch/x86/kernel/kprobes/core.c > @@ -625,7 +625,7 @@ static int prepare_emulation(struct kprobe *p, struct insn *insn) > /* 1 byte conditional jump */ > p->ainsn.emulate_op = kprobe_emulate_jcc; > p->ainsn.jcc.type = opcode & 0xf; > - p->ainsn.rel32 = *(char *)insn->immediate.bytes; > + p->ainsn.rel32 = insn->immediate.value; > break; > case 0x0f: > opcode = insn->opcode.bytes[1]; > -- > 2.34.1 >
Nadav -- Masami Hiramatsu (Google) <mhiramat@kernel.org>
| |