From nobody Sun Dec 14 06:22:10 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C22E0C433F5 for ; Mon, 3 Oct 2022 07:31:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231270AbiJCHbS (ORCPT ); Mon, 3 Oct 2022 03:31:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40610 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231162AbiJCH3W (ORCPT ); Mon, 3 Oct 2022 03:29:22 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9571D192A8; Mon, 3 Oct 2022 00:20:05 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 368B360FA2; Mon, 3 Oct 2022 07:20:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4502FC433D6; Mon, 3 Oct 2022 07:20:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1664781604; bh=ltonZA8UZoNCsAd5zA4FMw/ErDxEugLoPZ/GEw1M94M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EYl2tKvB4i5v8REc5lPzRX5MqLFW2Mh8ZKim2uFRU2IvdpkR0KGY9Ftz0K/A2g+Dy cHkM9ZYkdctPkjRA4kNA7ihyDGiri4pRXYUIW2Ec5cz+R3iEYPhmmSMe7EpHxF3oEg THWRfESLaZQv+aFWYTLeEl2Uj6dTcq9KjQ5yCCPQ= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Nadav Amit , "Peter Zijlstra (Intel)" , stable@kernel.org Subject: [PATCH 5.15 80/83] x86/alternative: Fix race in try_get_desc() Date: Mon, 3 Oct 2022 09:11:45 +0200 Message-Id: <20221003070724.006942646@linuxfoundation.org> X-Mailer: git-send-email 2.37.3 In-Reply-To: <20221003070721.971297651@linuxfoundation.org> References: <20221003070721.971297651@linuxfoundation.org> User-Agent: quilt/0.67 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" From: Nadav Amit commit efd608fa7403ba106412b437f873929e2c862e28 upstream. I encountered some occasional crashes of poke_int3_handler() when kprobes are set, while accessing desc->vec. The text poke mechanism claims to have an RCU-like behavior, but it does not appear that there is any quiescent state to ensure that nobody holds reference to desc. As a result, the following race appears to be possible, which can lead to memory corruption. CPU0 CPU1 ---- ---- text_poke_bp_batch() -> smp_store_release(&bp_desc, &desc) [ notice that desc is on the stack ] poke_int3_handler() [ int3 might be kprobe's so sync events are do not help ] -> try_get_desc(descp=3D&bp_desc) desc =3D __READ_ONCE(bp_desc) if (!desc) [false, success] WRITE_ONCE(bp_desc, NULL); atomic_dec_and_test(&desc.refs) [ success, desc space on the stack is being reused and might have non-zero value. ] arch_atomic_inc_not_zero(&desc->refs) [ might succeed since desc points to stack memory that was freed and might be reused. ] Fix this issue with small backportable patch. Instead of trying to make RCU-like behavior for bp_desc, just eliminate the unnecessary level of indirection of bp_desc, and hold the whole descriptor as a global. Anyhow, there is only a single descriptor at any given moment. Fixes: 1f676247f36a4 ("x86/alternatives: Implement a better poke_int3_handl= er() completion scheme") Signed-off-by: Nadav Amit Signed-off-by: Peter Zijlstra (Intel) Cc: stable@kernel.org Link: https://lkml.kernel.org/r/20220920224743.3089-1-namit@vmware.com Signed-off-by: Greg Kroah-Hartman --- arch/x86/kernel/alternative.c | 45 +++++++++++++++++++++----------------= ----- 1 file changed, 23 insertions(+), 22 deletions(-) --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -1200,22 +1200,23 @@ struct bp_patching_desc { atomic_t refs; }; =20 -static struct bp_patching_desc *bp_desc; +static struct bp_patching_desc bp_desc; =20 static __always_inline -struct bp_patching_desc *try_get_desc(struct bp_patching_desc **descp) +struct bp_patching_desc *try_get_desc(void) { - /* rcu_dereference */ - struct bp_patching_desc *desc =3D __READ_ONCE(*descp); + struct bp_patching_desc *desc =3D &bp_desc; =20 - if (!desc || !arch_atomic_inc_not_zero(&desc->refs)) + if (!arch_atomic_inc_not_zero(&desc->refs)) return NULL; =20 return desc; } =20 -static __always_inline void put_desc(struct bp_patching_desc *desc) +static __always_inline void put_desc(void) { + struct bp_patching_desc *desc =3D &bp_desc; + smp_mb__before_atomic(); arch_atomic_dec(&desc->refs); } @@ -1248,15 +1249,15 @@ noinstr int poke_int3_handler(struct pt_ =20 /* * Having observed our INT3 instruction, we now must observe - * bp_desc: + * bp_desc with non-zero refcount: * - * bp_desc =3D desc INT3 + * bp_desc.refs =3D 1 INT3 * WMB RMB - * write INT3 if (desc) + * write INT3 if (bp_desc.refs !=3D 0) */ smp_rmb(); =20 - desc =3D try_get_desc(&bp_desc); + desc =3D try_get_desc(); if (!desc) return 0; =20 @@ -1310,7 +1311,7 @@ noinstr int poke_int3_handler(struct pt_ ret =3D 1; =20 out_put: - put_desc(desc); + put_desc(); return ret; } =20 @@ -1341,18 +1342,20 @@ static int tp_vec_nr; */ static void text_poke_bp_batch(struct text_poke_loc *tp, unsigned int nr_e= ntries) { - struct bp_patching_desc desc =3D { - .vec =3D tp, - .nr_entries =3D nr_entries, - .refs =3D ATOMIC_INIT(1), - }; unsigned char int3 =3D INT3_INSN_OPCODE; unsigned int i; int do_sync; =20 lockdep_assert_held(&text_mutex); =20 - smp_store_release(&bp_desc, &desc); /* rcu_assign_pointer */ + bp_desc.vec =3D tp; + bp_desc.nr_entries =3D nr_entries; + + /* + * Corresponds to the implicit memory barrier in try_get_desc() to + * ensure reading a non-zero refcount provides up to date bp_desc data. + */ + atomic_set_release(&bp_desc.refs, 1); =20 /* * Corresponding read barrier in int3 notifier for making sure the @@ -1440,12 +1443,10 @@ static void text_poke_bp_batch(struct te text_poke_sync(); =20 /* - * Remove and synchronize_rcu(), except we have a very primitive - * refcount based completion. + * Remove and wait for refs to be zero. */ - WRITE_ONCE(bp_desc, NULL); /* RCU_INIT_POINTER */ - if (!atomic_dec_and_test(&desc.refs)) - atomic_cond_read_acquire(&desc.refs, !VAL); + if (!atomic_dec_and_test(&bp_desc.refs)) + atomic_cond_read_acquire(&bp_desc.refs, !VAL); } =20 static void text_poke_loc_init(struct text_poke_loc *tp, void *addr,