[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMj1kXFn=pfJqTrMrnqT+OLi3UqAq0PieROQhbC2M3B2uAzyEQ@mail.gmail.com>
Date: Thu, 17 Oct 2024 12:31:31 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Mark Rutland <mark.rutland@....com>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Clement LE GOFFIC <clement.legoffic@...s.st.com>, Russell King <linux@...linux.org.uk>,
"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>, Kees Cook <kees@...nel.org>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Mark Brown <broonie@...nel.org>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com,
Antonio Borneo <antonio.borneo@...s.st.com>
Subject: Re: Crash on armv7-a using KASAN
On Thu, 17 Oct 2024 at 12:10, Mark Rutland <mark.rutland@....com> wrote:
>
> On Wed, Oct 16, 2024 at 09:00:22PM +0200, Linus Walleij wrote:
> > On Wed, Oct 16, 2024 at 10:55 AM Mark Rutland <mark.rutland@....com> wrote:
> >
> > > I believe that's necessary for the lazy TLB switch, at least for SMP:
> > >
> > > // CPU 0 // CPU 1
> > >
> > > << switches to task X's mm >>
> > >
> > > << creates kthread task Y >>
> > > << maps task Y's new stack >>
> > > << maps task Y's new shadow >>
> > >
> > > // Y switched out
> > > context_switch(..., Y, ..., ...);
> > >
> > > // Switch from X to Y
> > > context_switch(..., X, Y, ...) {
> > > // prev = X
> > > // next = Y
> > >
> > > if (!next->mm) {
> > > // Y has no mm
> > > // No switch_mm() here
> > > // ... so no check_vmalloc_seq()
> > > } else {
> > > // not taken
> > > }
> > >
> > > ...
> > >
> > > // X's mm still lacks Y's stack + shadow here
> > >
> > > switch_to(prev, next, prev);
> > > }
> > >
> > > ... so probably worth a comment that we're faulting in the new
> > > stack+shadow for for lazy tlb when switching to a task with no mm?
> >
> > Switching to a task with no mm == switching to a kernel daemon.
>
> A common misconception, but not always true:
>
> * A kernel thread can have an mm: see kthread_use_mm() and
> kthread_unuse_mm().
>
> * A user thread can lose its mm while exiting: see how do_exit() calls
> exit_mm(), and how hte task remains preemptible for a while
> thereafter.
>
> ... so we really do just mean "a task with no mm".
>
> > And those only use the kernel memory and relies on that always
> > being mapped in any previous mm context, right.
>
> A task with no mm only uses kernel memory. Anything it uses must be
> mapped in init_mm, but *might* not have been copied into every other mm,
> and hence might not be in the previous mm context as per the example
> above.
>
> > But where do we put that comment? In kernel/sched/core.c
> > context_switch()?
>
> I was trying to suggest we update the existing comment in switch_to() to
> be more explicit. e.g. expand the existing comment:
>
> @
> @ Do a dummy read from the new stack while running from the old one so
> @ that we can rely on do_translation_fault() to fix up any stale PMD
> @ entries covering the vmalloc region.
> @
>
> ... with:
>
> @
> @ For a non-lazy mm switch, check_vmalloc_seq() has ensured that
> @ that the active mm's page tables have mappings for the prev
> @ task's stack and the next task's stack.
> @
> @ For a lazy mm switch the active mm's page tables have mappings
> @ for the prev task's stack but might not have mappings for the
> @ new taks stack. Do a dummy read from the new stack while
task
> @ running from the old stack so that we can rely on
> @ do_translation_fault() to fix up any stale PMD entries
> @ covering the vmalloc region.
Might as well be more precise here, and say "populate missing PMD
entries covering the new task's stack in the old task's page tables"
> @
>
> Ard, does that sound good to you?
>
Yes.
Powered by blists - more mailing lists