linux-kernel - Re: [PATCH] mmu_notifiers: Notify on pte permission upgrades

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: <87wn0z98n7.fsf@nvidia.com>
Date:   Tue, 23 May 2023 10:43:17 +1000
From:   Alistair Popple <apopple@...dia.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, robin.murphy@....com,
        will@...nel.org, nicolinc@...dia.com,
        linux-arm-kernel@...ts.infradead.org, kvm@...r.kernel.org,
        jgg@...dia.com, John Hubbard <jhubbard@...dia.com>
Subject: Re: [PATCH] mmu_notifiers: Notify on pte permission upgrades


Sean Christopherson <seanjc@...gle.com> writes:

> On Tue, May 23, 2023, Alistair Popple wrote:
>> 
>> Sean Christopherson <seanjc@...gle.com> writes:

[...]

>> > 	 * If invalidate_range() is used to manage a non-CPU TLB with
>> > 	 * shared page-tables, it not necessary to implement the
>> > 	 * invalidate_range_start()/end() notifiers, as
>> > 	 * invalidate_range() already catches the points in time when an
>> > 	 * external TLB range needs to be flushed. For more in depth
>> > 	 * discussion on this see Documentation/mm/mmu_notifier.rst
>> >
>> > Even worse, this change may silently regress performance for secondary MMUs that
>> > haven't yet taken advantage of the event type, e.g. KVM will zap all of KVM's PTEs
>> > in response to the upgrade, instead of waiting until the guest actually tries to
>> > utilize the new protections.
>> 
>> Yeah, I like the idea of introducing a
>> ptep_set_access_flags_notify(). That way this won't regress performance
>> on platforms that don't need it. Note this isn't a new feature but
>> rather a bugfix. It's unclear to me why KVM on ARM hasn't already run
>> into this issue, but I'm no KVM expert. Thanks for the feedback.
>
> KVM manages its own page tables and so does its own TLB invalidations as needed,
> e.g. KVM can and does change KVM's stage-2 PTEs from read-only to read/write
> irrespective of mmu_notifiers.  I assume the SMMU issue arises only because the
> SMMU is reusing the host kernel's (stage-1?) page tables.

Argh, thanks. That makes sense. The SMMU issue arises because it is not
snooping CPU TLB invalidations and therefore relies entirely on notifier
callbacks to invalidate it's TLB. If it was snooping invalidations it
would observe the TLB invalidation ARM64 does in
ptep_set_access_flags()[1]. Now that I've figured out we can call
invalidate_range() under the PTL I think I can just add the notifier
call there.

[1] - https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/mm/fault.c?id=ae8373a5add4ea39f032563cf12a02946d1e3546#n229