linux-kernel - Re: [PATCH v3 1/2] mm/vmalloc: Add a safer version of find_vm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230908001805.GA4088026@google.com>
Date:   Fri, 8 Sep 2023 00:18:05 +0000
From:   Joel Fernandes <joel@...lfernandes.org>
To:     Uladzislau Rezki <urezki@...il.com>
Cc:     Lorenzo Stoakes <lstoakes@...il.com>, linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Hellwig <hch@...radead.org>,
        "Paul E. McKenney" <paulmck@...nel.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Zhen Lei <thunder.leizhen@...weicloud.com>,
        rcu@...r.kernel.org, Zqiang <qiang.zhang1211@...il.com>,
        stable@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v3 1/2] mm/vmalloc: Add a safer version of find_vm_area()
 for debug

On Thu, Sep 07, 2023 at 11:23:40AM +0200, Uladzislau Rezki wrote:
> On Thu, Sep 07, 2023 at 08:11:48AM +0100, Lorenzo Stoakes wrote:
[..]
> > Anyway, so TL;DR:-
> > 
> > 1. As we both agree, add a comment to explain why you need the spin trylock.
> > (there are no further steps :P)
> > 
> > And I don't believe this actually needs any further changes after this
> > discussion*, so if you fancy doing a follow up to that effect that will
> > suffice for me thanks!

Thanks.

> For PREEMPT_RT kernels we are not allowed to use "vmap parts" in non
> slepable context, this is just reality, because we use a sleep type of
> spinlock.
> 
> I am not sure how urgent we need this fix. But to me it looks like
> debuging and corner case. Probably i am wrong and miss something.
> But if it is correct, i would just bailout for RT kernel and rework
> later in a more proper way. For example implement a safe way of RCU
> scan but this is also another story.

Bailing out for RT kernel is insufficient, as we need the trylock() to avoid
self-deadlock as well for !PREEMPT_RT. Plus IIRC in the past there was a
opposition to special-casing PREEMPT_RT in code as well. Admittedly those
PREEMPT_RT cases were related to detecting preempt-disabled than a lock-held
section though.

We could optionally do a trylock() loop + bail out after certain number of
tries as well but that would compilicate the code a bit more and I am not
sure if it is worth it. Still if you guys feel strongly about doing something
like that, let me know and I can give it a try :).

thanks,

 - Joel