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;
From:   Nadav Amit <namit@vmware.com>
To:     "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
CC:     Peter Zijlstra <peterz@infradead.org>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ingo Molnar <mingo@kernel.org>,
        Suleiman Souhlal <suleiman@google.com>,
        bpf <bpf@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Borislav Petkov <bp@suse.de>, "x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH v3 2/2] x86/kprobes: Fix optprobe optimization check with
 CONFIG_RETHUNK
Thread-Topic: [PATCH v3 2/2] x86/kprobes: Fix optprobe optimization check with
 CONFIG_RETHUNK
Thread-Index: AQHZEDW18AwsQ28s7U+Yq+RD5eoSpw==
Date:   Thu, 15 Dec 2022 03:31:25 +0000
Message-ID: <5BCC1248-FC01-4EB2-BFB2-4BBDD9092045@vmware.com>
References: <166264927154.775585.16570756675363838701.stgit@devnote2>
 <166264929259.775585.14768855667710290362.stgit@devnote2>
In-Reply-To: <166264929259.775585.14768855667710290362.stgit@devnote2>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?q4Es3vzfxUkL0DLrjvmrkSGkB8EN7AO4GXJ9a1OcZWSy/ZUGFLv7oFQMikut?=
 =?us-ascii?Q?Civq6Wt5tqda0RYVYl8t2lJCp81PUSXfVjWOCYujV7Gsa19A71fKQdcHzQrs?=
 =?us-ascii?Q?X1QN5dNQxPxI//mIBGKgB07kuJsXMCbTp3t019DkTbhZtMqfOMNKvJfCwXke?=
 =?us-ascii?Q?PaVKF5vz7BBk+UmC8JGmnaszMQexbgf++IUuWfOAB4qLDFT48YQ+LW3z0Sv5?=
 =?us-ascii?Q?iOmKrcfunEAmIWNUYWKipaGHt40gn9IO97EbiMuYVPkJ6DxlvqXBiHSBFFBR?=
 =?us-ascii?Q?soVOXD8Kn8esOm6RZFo9un2/yvJdbXXJLkBTj6KVqkL45agwIN+UHFEBzNKu?=
 =?us-ascii?Q?M/oXdxHTgAWxmp6jUWvMEU/wlBLPuMY164COdW3OvGcWf3pSbPWBnppGt67q?=
 =?us-ascii?Q?mp37xDINRs11REbTlisqB/GM5ommk1igY5r9BBFXGHKW3cMnILetPVS4ut0q?=
 =?us-ascii?Q?8CRvjvYCttflsJX/qvDL4GnHYilkGLDXyN4eCNsg6u3YA4rT35rZ5pVw/TCj?=
 =?us-ascii?Q?eXBuBTLdKfA+wWIMjptp7UaB+JkhOZfHFvluxWZORSlByXO4nVXorZyut6l6?=
 =?us-ascii?Q?vDJ8srdYIMwywGZoaLynHfQo5K4y/kgnI2walRRDBxbqZ72Et4kBLsibQieI?=
 =?us-ascii?Q?F0dnzu54Zivhb+Vl6n8wuOCl9Be9K32VnHTQRzLpdUIka7DqCaH/WdilXBQo?=
 =?us-ascii?Q?qENGbPR3UQekiXNv7Wh1bRWLkH09nKo6q2GEQqS/DXt2hsFlBcmf3ZQ3kmsj?=
 =?us-ascii?Q?Nra5ZB3sNGJ9GQCIxkZB8LicHg6MivmHbNgLDzP/w+Ho0GECAbYUksLPFwnC?=
 =?us-ascii?Q?QpslFODtihtGwW+9YfJySk22TbqME1jcJId3lenkNNQueVkVW8femWcxrzzU?=
 =?us-ascii?Q?Ek3osfvzlqx4glaNC9ydt0NlrXYRtZ3si/yEovU7SfuJnevL0oKVOFlIzGbg?=
 =?us-ascii?Q?7EfS+brbfxoAtgcKftc+ox57p2rY6BujjucAgr9QQpVj4yaW0QDai7RmY1Lh?=
 =?us-ascii?Q?CeMjuQqbdl6xv8YdL19eqkRwMMR/Y3gbtrdVihl5Y3sJcE9cl4lN5ayV9k0p?=
 =?us-ascii?Q?zCq9POwLrkGV/kM/QvGqdlNTBY2yjRdLTxCXvGi6QQ8/wYY31JDoCa6QdNse?=
 =?us-ascii?Q?atnApOHkjKMifikpA3QgP4Xg8G4V/h6cynD+D58l1Y/L/woVSKX8KVvbFOd8?=
 =?us-ascii?Q?mZhhsIOhGGB1Q/wcjYeVUjsH4eAOlF2Ah0PE+RxjxSQ71m+Okm7d13pcWwq+?=
 =?us-ascii?Q?qPiBpA2/mDmuIpNtCmzddXyO8IhwF9xmnRO1gcbKK8lreq4c/1GWZj+ZYz7s?=
 =?us-ascii?Q?Z8wVSoD61p7JaRxajSvms8Z3sfkZchw81XeKLBeVmX9FiClKw48UiNRY9Vo4?=
 =?us-ascii?Q?D7i05MYqhuES7VhScZYTwz64Q6WRBJmGnHWqjXWJQ0EqJSIvNLFOuApdR80S?=
 =?us-ascii?Q?PI7tg/LkcLfVTsPP8MqzcasGmLj21DiS+Q9dZispC88S9JXJ3RNqlCOTyO8h?=
 =?us-ascii?Q?XeTqzv/PxWkEuqnWBE+TV2x/Er4w8KN6ruXHh7FlCPdYPFEN+91hjQowCZkr?=
 =?us-ascii?Q?mNDTGRC3iU2zcE1C39/K1ZkFNPyu7xDNN/5SCF7g?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <B85B81A94E0BEB4EAC19177E6CC73BDC@namprd05.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: vmware.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BY3PR05MB8531.namprd05.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2a45c6ea-e4ba-4a83-08b7-08dade4cd85a
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Dec 2022 03:31:25.4381
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b39138ca-3cee-4b4a-a4d6-cd83d9dd62f0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 6Lt3KUPtpBp0IQBTBU3KToZ2mM0Gmw3kGpVxGpufPDQwfN7Tn5uve4Z5jUDaGT0P8TUW/Qqv+J+GgK1H1W+nAw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR05MB6069
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,SPF_NONE autolearn=no
        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


> On Sep 8, 2022, at 8:01 AM, Masami Hiramatsu (Google) <mhiramat@kernel.or=
g> wrote:
>=20
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>=20
> Since the CONFIG_RETHUNK and CONFIG_SLS will use INT3 for stopping
> speculative execution after function return, kprobe jump optimization
> always fails on the functions with such INT3 inside the function body.
> (It already checks the INT3 padding between functions, but not inside
> the function)
>=20
> To avoid this issue, as same as kprobes, check whether the INT3 comes
> from kgdb or not, and if so, stop decoding and make it fail. The other
> INT3 will come from CONFIG_RETHUNK/CONFIG_SLS and those can be
> treated as a one-byte instruction.
>=20
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Fixes: e463a09af2f0 ("x86: Add straight-line-speculation mitigation")
> Cc: stable@vger.kernel.org
> ---
> arch/x86/kernel/kprobes/opt.c |   28 ++++++++--------------------
> 1 file changed, 8 insertions(+), 20 deletions(-)
>=20
> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.=
c
> index e6b8c5362b94..e57e07b0edb6 100644
> --- a/arch/x86/kernel/kprobes/opt.c
> +++ b/arch/x86/kernel/kprobes/opt.c
> @@ -15,6 +15,7 @@
> #include <linux/extable.h>
> #include <linux/kdebug.h>
> #include <linux/kallsyms.h>
> +#include <linux/kgdb.h>
> #include <linux/ftrace.h>
> #include <linux/objtool.h>
> #include <linux/pgtable.h>
> @@ -279,19 +280,6 @@ static int insn_is_indirect_jump(struct insn *insn)
> return ret;
> }
>=20
> -static bool is_padding_int3(unsigned long addr, unsigned long eaddr)
> -{
> - unsigned char ops;
> -
> - for (; addr < eaddr; addr++) {
> - if (get_kernel_nofault(ops, (void *)addr) < 0 ||
> -    ops !=3D INT3_INSN_OPCODE)
> - return false;
> - }
> -
> - return true;
> -}
> -
> /* Decode whole function to ensure any instructions don't jump into targe=
t */
> static int can_optimize(unsigned long paddr)
> {
> @@ -334,15 +322,15 @@ static int can_optimize(unsigned long paddr)
> ret =3D insn_decode_kernel(&insn, (void *)recovered_insn);
> if (ret < 0)
> return 0;
> -
> +#ifdef CONFIG_KGDB
> /*
> - * In the case of detecting unknown breakpoint, this could be
> - * a padding INT3 between functions. Let's check that all the
> - * rest of the bytes are also INT3.
> + * If there is a dynamically installed kgdb sw breakpoint,
> + * this function should not be probed.
> */
> - if (insn.opcode.bytes[0] =3D=3D INT3_INSN_OPCODE)
> - return is_padding_int3(addr, paddr - offset + size) ? 1 : 0;
> -
> + if (insn.opcode.bytes[0] =3D=3D INT3_INSN_OPCODE &&
> +    kgdb_has_hit_break(addr))
> + return 0;
> +#endif
> /* Recover address */
> insn.kaddr =3D (void *)addr;
> insn.next_byte =3D (void *)(addr + insn.length);

Hi Masami,

I encountered a similar issue with can_probe(). I see that your
patches were not upstreamed, at least to 6.1.

So I was wondering whether it they are going to be upstreamed, and
whether you want also to update can_probe().

Thanks,
Nadav=