From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Michael Schmitz <schmitzmic@gmail.com>
Cc: linux-m68k@vger.kernel.org, Guenter Roeck <linux@roeck-us.net>,
Finn Thain <fthain@linux-m68k.org>,
Al Viro <viro@zeniv.linux.org.uk>,
stable@kernel.org, linux-m68k@lists.linux-m68k.org
Subject: Re: [PATCH RFC] m68k: fix spinlock race in kernel thread creation
Date: Wed, 8 May 2024 13:53:06 +0200 [thread overview]
Message-ID: <CAMuHMdVMXdMOOf5idJVinO7TUWDH+Gf=JXw2-=p87YBYy+xARA@mail.gmail.com> (raw)
In-Reply-To: <20240411033631.16335-1-schmitzmic@gmail.com>
On Thu, Apr 11, 2024 at 5:36 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Context switching does take care to retain the correct lock
> owner across the switch from 'prev' to 'next' tasks. This does
> rely on interrupts remaining disabled for the entire duration
> of the switch.
>
> This condition is guaranteed for normal process creation and
> context switching between already running processes, because
> both 'prev' and 'next' already have interrupts disabled in their
> saved copies of the status register.
>
> The situation is different for newly created kernel threads.
> The status register is set to PS_S in copy_thread(), which does
> leave the IPL at 0. Upon restoring the 'next' thread's status
> register in switch_to() aka resume(), interrupts then become
> enabled prematurely. resume() then returns via ret_from_kernel_thread()
> and schedule_tail() where run queue lock is released (see
> finish_task_switch() and finish_lock_switch()).
>
> A timer interrupt calling scheduler_tick() before the lock is
> released in finish_task_switch() will find the lock already taken,
> with the current task as lock owner. This causes a spinlock
> recursion warning as reported by Guenter Roeck.
>
> As far as I can ascertain, this race has been opened in commit
> 533e6903bea0 ("m68k: split ret_from_fork(), simplify kernel_thread()")
> but I haven't done a detailed study of kernel history so it
> may well predate that commit.
>
> Interrupts cannot be disabled in the saved status register copy
> for kernel threads (init will complain about interrupts disabled
> when finally starting user space). Disable interrupts temporarily
> when switching the tasks' register sets in resume().
>
> Note that a simple oriw 0x700,%sr after restoring sr is not enough
> here - this leaves enough of a race for the 'spinlock recursion'
> warning to still be observed.
>
> Tested on ARAnyM and qemu (Quadra 800 emulation).
>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Finn Thain <fthain@linux-m68k.org>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: stable@kernel.org # 3.6
> Cc: linux-m68k@lists.linux-m68k.org
Reported-by: Guenter Roeck <linux@roeck-us.net>
> Link: https://lore.kernel.org/all/07811b26-677c-4d05-aeb4-996cd880b789@roeck-us.net
s/Link/Closes/
> Fixes: 533e6903bea0 ("m68k: split ret_from_fork(), simplify kernel_thread()")
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
i.e. will queue in the m68k tree for v6.10 (with above tags fixed).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
prev parent reply other threads:[~2024-05-08 11:53 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-11 3:36 [PATCH RFC] m68k: fix spinlock race in kernel thread creation Michael Schmitz
2024-04-11 12:10 ` Guenter Roeck
2024-05-08 11:53 ` Geert Uytterhoeven [this message]
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='CAMuHMdVMXdMOOf5idJVinO7TUWDH+Gf=JXw2-=p87YBYy+xARA@mail.gmail.com' \
--to=geert@linux-m68k.org \
--cc=fthain@linux-m68k.org \
--cc=linux-m68k@lists.linux-m68k.org \
--cc=linux-m68k@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=schmitzmic@gmail.com \
--cc=stable@kernel.org \
--cc=viro@zeniv.linux.org.uk \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).