Messages in this thread | | | Date | Wed, 15 Feb 2023 08:57:14 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH -tip] kprobes/x86: Remove unneeded casting from immediate value |
| |
On Fri, 10 Feb 2023 02:01:54 +0200 Nadav Amit <nadav.amit@gmail.com> wrote:
> > On 2/9/23 5:36 PM, Masami Hiramatsu (Google) wrote: > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Remove unneeded casting from immediate value assignments for relative > > jump offset. Since the immediate values in the 'insn' data structure are > > assigned from immediate bytes as a signed value to insn.immediate.value > > by insn_field_set(). Thus, if we need to access that value as a signed > > value (in this kprobe's case), we don't need to cast it. > > This is a kind of clean up (should not change behavior) follows Nadav's > > bugfix. > > > > Link: https://lore.kernel.org/all/20230208071708.4048-1-namit@vmware.com/ > > > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > --- > > arch/x86/kernel/kprobes/core.c | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c > > index 695873c0f50b..2440e736d0e4 100644 > > --- a/arch/x86/kernel/kprobes/core.c > > +++ b/arch/x86/kernel/kprobes/core.c > > @@ -607,19 +607,19 @@ static int prepare_emulation(struct kprobe *p, struct insn *insn) > > case 0xe8: /* near call relative */ > > p->ainsn.emulate_op = kprobe_emulate_call; > > if (insn->immediate.nbytes == 2) > > - p->ainsn.rel32 = *(s16 *)&insn->immediate.value; > > + p->ainsn.rel32 = insn->immediate.value; > > else > > - p->ainsn.rel32 = *(s32 *)&insn->immediate.value; > > + p->ainsn.rel32 = insn->immediate.value; > > Hmm.. I don't get it. What the purpose of keeping the duplicated code > (after your change)? >
Oops, good catch! I think p->ainsn.rel32 can be set without any check in general. Let me update it.
Thank you!
-- Masami Hiramatsu (Google) <mhiramat@kernel.org>
| |