linux-kernel - Re: [PATCH v2 4/6] mm: lock vma explicitly before doing vm_flags_reset and vm_flags_reset

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: <CAJuCfpFYq4yyj0=nW0iktoH0dma-eFhw1ni7v9R-fCsYH7eQ3Q@mail.gmail.com>
Date:   Wed, 2 Aug 2023 11:09:37 -0700
From:   Suren Baghdasaryan <surenb@...gle.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     akpm@...ux-foundation.org, jannh@...gle.com, willy@...radead.org,
        liam.howlett@...cle.com, david@...hat.com, peterx@...hat.com,
        ldufour@...ux.ibm.com, vbabka@...e.cz, michel@...pinasse.org,
        jglisse@...gle.com, mhocko@...e.com, hannes@...xchg.org,
        dave@...olabs.net, hughd@...gle.com, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org, stable@...r.kernel.org
Subject: Re: [PATCH v2 4/6] mm: lock vma explicitly before doing
 vm_flags_reset and vm_flags_reset_once

On Wed, Aug 2, 2023 at 10:49 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Tue, 1 Aug 2023 at 15:07, Suren Baghdasaryan <surenb@...gle.com> wrote:
> >
> >       To make locking more visible, change these
> > functions to assert that the vma write lock is taken and explicitly lock
> > the vma beforehand.
>
> So I obviously think this is a good change, but the fact that it
> touched driver files makes me go "we're still doing something wrong".
>
> I'm not super-happy with hfi1_file_mmap() doing something like
> vma_start_write(), in that I *really* don't think drivers should ever
> have to think about issues like this.
>
> And I think it's unnecessary.  This is the mmap op in the
> hfi1_file_ops, and I think that any actual mmap() code had _better_
> had locked the new vma before asking any driver to set things up (and
> the assert would catch it if somebody didn't).
>
> I realize that it doesn't hurt in a technical sense, but I think
> having drivers call these VM-internal subtle locking functions does
> hurt in a maintenance sense, so we should make sure to not have it.

Ok, IOW the vma would be already locked before mmap() is called...
Just to confirm, you are suggesting to remove vma_start_write() call
from hfi1_file_mmap() and let vm_flags_reset() generate an assertion
if it's ever called with an unlocked vma, correct?

>
>                    Linus