lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 21 Aug 2023 07:38:32 +0200
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     Mateusz Guzik <mjguzik@...il.com>, akpm@...ux-foundation.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH] mm: remove unintentional voluntary preemption in get_mmap_lock_carefully

On Mon, 21 Aug 2023 at 06:55, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> I do think that maybe we should then re-introduce the might_sleep() in
> some actually appropriate place in the page fault path, which might be
> 'handle_mm_fault()'.

Just to clarify: if we do it in handle_mm_fault(), we should obviously
do it only for kernel faults - just to avoid the unnecessary
preemption with lock held for the normal user space case.

That said, I don't love the handle_mm_fault() option, I just think it
would be lovely if we had that test in a generic place. Sadly, we
don't seem to have any such thing other than handle_mm_fault().

The reason we shouldn't do this for user space faults are fine is
because not only are they obviously always sleepable, but they also
reschedule on return to user space. So neither the debugging nor the
preemption makes sense there.

That's also why the per-vma locking paths don't need this - we only do
per-vma locking for user space faults.

So it really is only "kernel faults" it makes sense for, and they are
rare enough that I think the preemption point issue might be moot. But
it might still be better to just do this all as a special kernel fault
case in the exception handler - even if it's then
architecture-specific (like it always was before commit c2508ec5a58d).

              Linus

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ