linux-kernel - Re: [PATCH 2/2] arm64: 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: <ZHaCAJI+OgIfDWSx@nvidia.com>
Date:   Tue, 30 May 2023 20:08:48 -0300
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Robin Murphy <robin.murphy@....com>,
        Alistair Popple <apopple@...dia.com>,
        Andrew Morton <akpm@...ux-foundation.org>, will@...nel.org,
        catalin.marinas@....com, linux-mm@...ck.org,
        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
Subject: Re: [PATCH 2/2] arm64: Notify on pte permission upgrades

On Tue, May 30, 2023 at 02:44:40PM -0700, Sean Christopherson wrote:
> > KVM already has locking for invalidate_start/end - it has to check
> > mmu_notifier_retry_cache() with the sequence numbers/etc around when
> > it does does hva_to_pfn()
> > 
> > The bug is that the kvm_vcpu_reload_apic_access_page() path is
> > ignoring this locking so it ignores in-progress range
> > invalidations. It should spin until the invalidation clears like other
> > places in KVM.
> > 
> > The comment is kind of misleading because drivers shouldn't be abusing
> > the iommu centric invalidate_range() thing to fix missing locking in
> > start/end users. :\
> > 
> > So if KVM could be fixed up we could make invalidate_range defined to
> > be an arch specific callback to synchronize the iommu TLB.
> 
> And maybe rename invalidate_range() and/or invalidate_range_{start,end}() to make
> it super obvious that they are intended for two different purposes?  E.g. instead
> of invalidate_range(), something like invalidate_secondary_tlbs().

Yeah, I think I would call it invalidate_arch_secondary_tlb() and
document it as being an arch specific set of invalidations that match
the architected TLB maintenance requrements. And maybe we can check it
more carefully to make it be called in less places. Like I'm not sure
it is right to call it from invalidate_range_end under this new
definition..

> FWIW, PPC's OpenCAPI support (drivers/misc/ocxl/link.c) also uses invalidate_range().
> Though IIUC, the use case is the same as a "traditional" IOMMU, where a device can
> share the CPU's page tables, so maybe the devices can be considered IOMMUs in practice,
> if not in name?

OpenCAPI is an IOMMU HW for sure. PPC just doesn't have integration
with the drivers/iommu infrastructure.

> I have patches coded up.  Assuming testing goes well, I'll post them regardless
> of the OCXL side of things.  I've disliked KVM's one-off use of invalidate_range()
> for a long time, this is a good excuse to get rid of it before KVM gains more usage.

Nice!

Thanks,
Jason