Date:   Thu, 2 Mar 2023 16:36:03 -0500
From:   Steven Rostedt <rostedt@goodmis.org>
To:     John Stultz <jstultz@google.com>
Cc:     LKML <linux-kernel@vger.kernel.org>, Wei Wang <wvw@google.com>,
        Midas Chien <midaschieh@google.com>,
        Kees Cook <keescook@chromium.org>,
        Anton Vorontsov <anton@enomsg.org>,
        "Guilherme G. Piccoli" <gpiccoli@igalia.com>,
        Tony Luck <tony.luck@intel.com>, kernel-team@android.com,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Subject: Re: [PATCH] pstore: Revert pmsg_lock back to a normal mutex
Message-ID: <20230302163603.223313ba@gandalf.local.home>
In-Reply-To: <20230302163253.541ac3a8@gandalf.local.home>
References: <20230302062741.483079-1-jstultz@google.com>
        <20230302082414.77613351@gandalf.local.home>
        <CANDhNCo4ruC4pP+iDe49b3e1nAcWtYQj4bx82+oZhyLFYkdFJQ@mail.gmail.com>
        <20230302152103.2618f1b7@gandalf.local.home>
        <20230302163253.541ac3a8@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 2 Mar 2023 16:32:53 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

>    CPU1					CPU2
>    ----					----
> rt_mutex_slowlock_block() {
>   raw_spin_lock_irq(wait_lock);
>   owner = rt_mutex_owner();
>   raw_spin_unlock_irq(wait_lock);
>
>   rtmutex_spin_on_owner(owner) {

I just noticed there's an rcu_read_lock() here around the loop.
I'm guessing that's to keep this race from happening.
Would have been nice to have a comment there stating such.

-- Steve

>     owner = rt_mutex_owner();
> 
>     [ task preempted! (could also be a long interrupt) ]
> 
> 				   owner releases lock and exits
> 				   owner is freed
> 
>     [ task resumes ]
> 
>     if (!owner_on_cpu(owner)
> 
>       READ_ONCE(owner->on_cpu)
>      *** BOOM invalid pointer dereference ***