[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <EECE6FD7-FC3D-40E2-A957-151A1BE2B2F9@vmware.com>
Date: Tue, 20 Sep 2022 23:13:42 +0000
From: Nadav Amit <namit@...are.com>
To: Nadav Amit <nadav.amit@...il.com>
CC: Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...hat.com>,
"stable@...r.kernel.org" <stable@...r.kernel.org>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>,
"Steven Rostedt (Google)" <rostedt@...dmis.org>
Subject: Re: [PATCH] x86/alternative: fix race in try_get_desc()
On Sep 20, 2022, at 3:47 PM, Nadav Amit <nadav.amit@...il.com> wrote:
> From: Nadav Amit <namit@...are.com>
>
> 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=&bp_desc)
> desc = __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. ]
>
> I encountered some occasional crashes of poke_int3_handler() when
> kprobes are set, while accessing desc->vec. The analysis has been done
> offline and I did not corroborate the cause of the crashes. Yet, it
> seems that this race might be the root cause.
>
> 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 on the stack.
> Anyhow, there is only a single descriptor at any given moment.
>
> Fixes: 1f676247f36a4 ("x86/alternatives: Implement a better poke_int3_handler() completion scheme"
A bracket is mistakenly missing after the patch title. Sorry.
Powered by blists - more mailing lists