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]
Date: Sun, 21 Jan 2024 16:23:35 +0100
From: Matthieu Baerts <matttbe@...nel.org>
To: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, x86@...nel.org,
 Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Peter Zijlstra <peterz@...radead.org>,
 linux-kernel@...r.kernel.org, Huacai Chen <chenhuacai@...ngson.cn>,
 Jinyang He <hejinyang@...ngson.cn>, Tiezhu Yang <yangtiezhu@...ngson.cn>,
 "Naveen N . Rao" <naveen.n.rao@...ux.ibm.com>
Subject: Re: [PATCH -tip v2] x86/kprobes: Drop removed INT3 handling code

Hi Masami, Steven,

On 21/01/2024 10:05, Masami Hiramatsu (Google) wrote:
> On Sun, 21 Jan 2024 11:28:52 +0900
> Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:
> 
>> On Sat, 20 Jan 2024 17:05:17 -0500
>> Steven Rostedt <rostedt@...dmis.org> wrote:
>>
>>> On Sat, 20 Jan 2024 18:44:38 +0100
>>> Matthieu Baerts <matttbe@...nel.org> wrote:
>>>
>>>>
>>>> I'm sorry to reply on a patch that is more than one year old, but in
>>>
>>> No problem, I've done the same.
>>
>> Yeah, thanks for reporting! I realized the problem.

Thank you both for your quick reply, very useful explanations, analysis
and patch!

(...)

> So this another solution is already done. I think we need to add the
> ghost INT3 check in the exc_int3() as follows;
> 
> Thank you,
> 
> From add8cf7da99cdb096a0d6765b3dc5de9a3ea3019 Mon Sep 17 00:00:00 2001
> From: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
> Date: Sun, 21 Jan 2024 17:16:50 +0900
> Subject: [PATCH] x86: Fixup from the removed INT3 if it is unhandled
> 
> INT3 is used not only for software breakpoint, but also self modifying
> code on x86 in the kernel. For example, jump_label, function tracer etc.
> Those may not handle INT3 after removing it but not waiting for
> synchronizing CPUs enough. Since such 'ghost' INT3 is not handled by
> anyone because they think it has been removed already.
> Recheck there is INT3 on the exception address and if not, ignore it.
> 
> Note that previously kprobes does the same thing by itself, but that is
> not a good location to do that because INT3 is commonly used. Do it at
> the common place so that it can handle all 'ghost' INT3.

I just tested it, and I was able to run pings for 3h without any issues!

While at it, and just to be on the safe side, I also re-run the tests
after having added a "pr_warn()" -- I know, using printk(), especially
when talking to you... but I was not sure what was safe to use at this
place in the code :) -- before returning "true" in the new function you
added, and we can see that the crash is avoided thanks to the new code:

[   27.422518] traps: crash avoided, addr=18446744071882050317
[   27.426182] traps: crash avoided, addr=18446744071882050317

[  370.483208] traps: crash avoided, addr=18446744071882075656
[  370.485066] traps: crash avoided, addr=18446744071882075656
[  370.485084] traps: crash avoided, addr=18446744071882075656

[  592.866416] traps: crash avoided, addr=18446744071882075656
[  592.867937] traps: crash avoided, addr=18446744071882075656

[  980.988342] traps: crash avoided, addr=18446744071882050317
[  980.989866] traps: crash avoided, addr=18446744071882050317

(from my VM running with 2 CPU cores)


Again, thank you for the fix!

(Just in case you need it:)

Tested-by: Matthieu Baerts <matttbe@...nel.org>

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ