linux-kernel - Re: [PATCH] x86/kprobes: Fix 1 byte conditional jump target

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]

Message-ID: <B790D31A-A0BE-4707-BA01-398B5668C3D8@vmware.com>
Date:   Wed, 8 Feb 2023 06:34:27 +0000
From:   Nadav Amit <namit@...are.com>
To:     Dave Hansen <dave.hansen@...el.com>
CC:     "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 ML <x86@...nel.org>,
        kernel list <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] x86/kprobes: Fix 1 byte conditional jump target

> On Feb 7, 2023, at 5:33 PM, Dave Hansen <dave.hansen@...el.com> wrote:
> 
> !! External Email
> 
> On 2/7/23 07:21, Masami Hiramatsu (Google) wrote:
>>> Nadav, can you update your patch to assign immediate.value directly?
>> BTW, there are many similar casts around there. I'll fix those too.
>> If we need to be more conservative,
> 
> Let's focus on fixing the known bug first, please.  Cleanups can come later.

Thank you Dave. That was my take too following your email.

I certainly did not pay attention to the fact that sign extension has already
been done in insn_get_immediate() before Masami pointed it out. So, the comment
in insn_get_immediate() should also be updated to note that the immediate
is sign-*extended* in *all* cases (instead of sign-expanded in most cases. :) )

As you said, I guess the change you and Masami proposed can be done on top of
this patch, which is (and was) only intended to fix the bug, and should
therefore go into 6.2.