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: <87jzwz97cv.fsf@nvidia.com>
Date:   Tue, 23 May 2023 11:12:50 +1000
From:   Alistair Popple <apopple@...dia.com>
To:     John Hubbard <jhubbard@...dia.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,
        kvmarm@...ts.cs.columbia.edu, jgg@...dia.com
Subject: Re: [PATCH] mmu_notifiers: Notify on pte permission upgrades


John Hubbard <jhubbard@...dia.com> writes:

> On 5/21/23 23:37, Alistair Popple wrote:
>> Some architectures, specifically ARM and perhaps Sparc and IA64,
>> require TLB invalidates when upgrading pte permission from read-only
>> to read-write.
>> The current mmu_notifier implementation assumes that upgrades do not
>> need notifications. Typically though mmu_notifiers are used to
>> implement TLB invalidations for secondary MMUs that comply with the
>> main CPU architecture.
>> Therefore if the main CPU architecture requires an invalidation for
>> permission upgrade the secondary MMU will as well and an mmu_notifier
>> should be sent for the upgrade.
>> Currently CPU invalidations for permission upgrade occur in
>> ptep_set_access_flags(). Unfortunately MMU notifiers cannot be called
>> directly from this architecture specific code as the notifier
>> callbacks can sleep, and ptep_set_access_flags() is usually called
>> whilst holding the PTL spinlock. Therefore add the notifier calls
>> after the PTL is dropped and only if the PTE actually changed. This
>> will allow secondary MMUs to obtain an updated PTE with appropriate
>> permissions.
>> This problem was discovered during testing of an ARM SMMU
>> implementation that does not support broadcast TLB maintenance
>> (BTM). In this case the SMMU driver uses notifiers to issue TLB
>> invalidates. For read-only to read-write pte upgrades the SMMU
>> continually returned a read-only PTE to the device, even though the
>> CPU had a read-write PTE installed.
>> Sending a mmu notifier event to the SMMU driver fixes the problem by
>> flushing secondary TLB entries. A new notifier event type is added so
>> drivers may filter out these invalidations if not required. Note a
>> driver should never upgrade or install a PTE in response to this mmu
>> notifier event as it is not synchronised against other PTE operations.
>> Signed-off-by: Alistair Popple <apopple@...dia.com>
>> ---
>>   include/linux/mmu_notifier.h |  6 +++++
>>   mm/hugetlb.c                 | 24 ++++++++++++++++++-
>>   mm/memory.c                  | 45 ++++++++++++++++++++++++++++++++++--
>>   3 files changed, 72 insertions(+), 3 deletions(-)
>
> Point of order:
>
> What is this based on? It would really help if you would either use --base
> with git-format-patch, or else just mention the base somewhere. Otherwise,
> actually applying the patch takes some hunting around...in particular for
> older stuff. This is from Feb, 2023, before hugetlb.c got converted to
> folios, right?

Yes, my bad. This is based on v6.2. I had meant to rebase it prior to
sending but forgot. Based on the other review comments though I will be
reworking this to put the notifier call in ptep_set_access_flags() on
Arm anyway.

> thanks,