linux-kernel - Re: [PATCH 09/19] mm: New follow

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: <20240815172445.GK2032816@nvidia.com>
Date: Thu, 15 Aug 2024 14:24:45 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Peter Xu <peterx@...hat.com>
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Sean Christopherson <seanjc@...gle.com>,
	Oscar Salvador <osalvador@...e.de>,
	Axel Rasmussen <axelrasmussen@...gle.com>,
	linux-arm-kernel@...ts.infradead.org, x86@...nel.org,
	Will Deacon <will@...nel.org>, Gavin Shan <gshan@...hat.com>,
	Paolo Bonzini <pbonzini@...hat.com>, Zi Yan <ziy@...dia.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Ingo Molnar <mingo@...hat.com>,
	Alistair Popple <apopple@...dia.com>,
	Borislav Petkov <bp@...en8.de>,
	David Hildenbrand <david@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>, kvm@...r.kernel.org,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Alex Williamson <alex.williamson@...hat.com>,
	Yan Zhao <yan.y.zhao@...el.com>
Subject: Re: [PATCH 09/19] mm: New follow_pfnmap API

On Thu, Aug 15, 2024 at 01:21:01PM -0400, Peter Xu wrote:
> > Why? Either the function only returns PFN map no-struct page things or
> > it returns struct page stuff too, in which case why bother to check
> > the VMA flags if the caller already has to be correct for struct page
> > backed results?
> > 
> > This function is only safe to use under the proper locking, and under
> > those rules it doesn't matter at all what the result is..
> 
> Do you mean we should drop the PFNMAP|IO check?

Yeah

>  I didn't see all the
> callers to say that they won't rely on proper failing of !PFNMAP&&!IO vmas
> to work alright.  So I assume we should definitely keep them around.

But as before, if we care about this we should be using vm_normal_page
as that is sort of abusing the PFNMAP flags.

> >   Any physical address obtained through this API is only valid while
> >   the @follow_pfnmap_args. Continuing to use the address after end(),
> >   without some other means to synchronize with page table updates
> >   will create a security bug.
> 
> Some misuse on wordings here (e.g. we don't return PA but PFN), and some
> sentence doesn't seem to be complete.. but I think I get the "scary" part
> of it.  How about this, appending the scary part to the end?
> 
>  * During the start() and end() calls, the results in @args will be valid
>  * as proper locks will be held.  After the end() is called, all the fields
>  * in @follow_pfnmap_args will be invalid to be further accessed.  Further
>  * use of such information after end() may require proper synchronizations
>  * by the caller with page table updates, otherwise it can create a
>  * security bug.

I would specifically emphasis that the pfn may not be used after
end. That is the primary mistake people have made.

They think it is a PFN so it is safe.

> It sounds like we need some mmu notifiers when mapping the IOMMU pgtables,
> as long as there's MMIO-region / P2P involved.  It'll make sure when
> tearing down the BAR mappings, the devices will at least see the same view
> as the processors.

I think the mmu notifiers can trigger too often for this to be
practical for DMA :(

Jason