From: Nadav Amit <namit@vmware.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>, X86 ML <x86@kernel.org>,
Dave Hansen <dave.hansen@linux.intel.com>,
Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>,
"# 5 . 10+" <stable@vger.kernel.org>,
"Steven Rostedt (Google)" <rostedt@goodmis.org>
Subject: Re: [PATCH] x86/alternative: fix race in try_get_desc()
Date: Wed, 21 Sep 2022 18:03:11 +0000 [thread overview]
Message-ID: <68350BC2-074F-4776-B5CD-1335311CC1D8@vmware.com> (raw)
In-Reply-To: <YyrCrN6RwibR505Z@hirez.programming.kicks-ass.net>
On Sep 21, 2022, at 12:52 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> ⚠ External Email
>
> On Tue, Sep 20, 2022 at 10:47:43PM +0000, Nadav Amit wrote:
>
>> 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.
>
> Because of text_mutex; indeed. No idea why I put that thing on the
> stack.
>
> I've done a few minor edits to your patch, but it otherwise looks good
> to me.
>
> ---
> Subject: x86/alternative: Fix race in try_get_desc()
> From: Nadav Amit <namit@vmware.com>
> Date: Tue, 20 Sep 2022 22:47:43 +0000
>
> From: Nadav Amit <namit@vmware.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.
Looks good to me. Thanks for improving my patch.
I just made one small mistake in commit message. It should say “hold
the whole descriptor as a global” in the line above.
I will send v2 with your changes and the updated commit message.
Thanks again.
next prev parent reply other threads:[~2022-09-21 18:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-20 22:47 [PATCH] x86/alternative: fix race in try_get_desc() Nadav Amit
2022-09-20 23:13 ` Nadav Amit
2022-09-21 7:52 ` Peter Zijlstra
2022-09-21 18:03 ` Nadav Amit [this message]
2022-09-28 6:57 ` [tip: x86/urgent] x86/alternative: Fix " tip-bot2 for Nadav Amit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=68350BC2-074F-4776-B5CD-1335311CC1D8@vmware.com \
--to=namit@vmware.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).