linux-kernel - Re: [RFC PATCH 0/2] Invalidate secondary IOMMU TLB on permission upgrade

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:   Wed, 21 Jun 2023 12:06:13 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Alistair Popple <apopple@...dia.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        Robin Murphy <robin.murphy@....com>, will@...nel.org,
        catalin.marinas@....com, linux-kernel@...r.kernel.org,
        nicolinc@...dia.com, linux-arm-kernel@...ts.infradead.org,
        kvm@...r.kernel.org, John Hubbard <jhubbard@...dia.com>,
        zhi.wang.linux@...il.com, Sean Christopherson <seanjc@...gle.com>
Subject: Re: [RFC PATCH 0/2] Invalidate secondary IOMMU TLB on permission
 upgrade

On Tue, Jun 20, 2023 at 09:18:24PM +1000, Alistair Popple wrote:

> 1. Add a call to mmu_notifier_invalidate_secondary_tlbs() to the arm64
>    version of ptep_set_access_flags().

I prefer we modify the whole thing to only call
mmu_notifier_arch_invalidate_secondary_tlbs() (note the arch in the
name) directly beside the arch's existing tlb invalidation, and we
only need to define this for x86 and ARM arches that have secondary
TLB using drivers - eg it is very much not a generic general purpose
mmu notifier that has any meaning outside arch code.

> This is what this RFC series does as it is the simplest
> solution. Arguably this call should be made by generic kernel code
> though to catch other platforms that need it.

But that is the whole point, the generic kernel cannot and does not
know the rules for TLB invalidation the platform model requires.

> It is unclear if mmu_notifier_invalidate_secondary_tlbs() should be
> called from mmu_notifier_range_end(). Currently it is, as an analysis
> of existing code shows most code doesn't explicitly invalidate
> secondary TLBs and relies on it being called as part of the end()
> call.

If you do the above we don't need to answer this question. Calling it
unconditionally at the arches tlbi points is always correct.

> To solve that we could add secondary TLB invalidation calls to the TLB
> batching code, but that adds complexity so I'm not sure it's worth it
> but would appreciate feedback.

It sounds like the right direction to me.. Batching is going to be
important, we don't need to different verions of batching logic. We
already call the notifier from the batch infrastructure anyhow.

This still fits with the above as batching goes down to the arch's
flush_tlb_range()/etc which can carry the batch to the notifier. We
can decide if we leave the notifier call in the core code and let the
arch elide it or push it to the arch so it the call is more
consistently placed.

eg we have arch_tlbbatch_flush() on x86 too that looks kind of
suspicious that is already really funky to figure out where the
notifier is actually called from. Calling from arch code closer to the
actual TLB flush would be alot easier to reason about.

Jason