Re: [PATCH] x86/kaslr: Revisit entropy when CONFIG_PCI_P2PDMA is enabled

[Balbir Singh]

On 2/7/25 06:59, Kees Cook wrote:
> On Thu, Feb 06, 2025 at 09:10:58AM +0100, Peter Zijlstra wrote:
>> On Thu, Feb 06, 2025 at 01:32:01PM +1100, Balbir Singh wrote:
>>> When CONFIG_PCI_P2PDMA is enabled, it maps the PFN's via a
>>> ZONE_DEVICE mapping using devm_memremap_pages(). The mapped
>>> virtual address range corresponds to the pci_resource_start()
>>> of the BAR address and size corresponding to the BAR length.
>>>
>>> When KASLR is enabled, the direct map range of the kernel is
>>> reduced to the size of physical memory plus additional padding.
>>> If the BAR address is beyond this limit, PCI peer to peer DMA
>>> mappings fail.
>>>
>>> Fix this by not shrinking the size of direct map when CONFIG_PCI_P2PDMA
>>> is enabled. This reduces the total available entropy, but it's
>>> better than the current work around of having to disable KASLR
>>> completely.
> 
> So, just to restate my understanding: this is about only the direct map
> (i.e. kaslr_region[0]). The notes (which I think should be left in the
> commit log) say that the entropy dropped from 49 TiB (46 bits) to 20 TiB
> (45 bits). If I'm reading right, the offset granularity is in PUD_SIZE
> (30 bits) steps, so the entropy is going from 16 bits to 15 bits. I don't
> see any general problem with that. Especially if the alternative is 0
> bits of entropy. :)
> 

Yes, this is about the direct map (kaslr_region[0]) and the data is from my
system which has 46 bits of physical address. On larger systems with LA57
the drop might be higher. I am happy to repost the patch with my testing notes
in the commit log, if you think it's useful to have in the commit log.

>> I'm thinking this CONFIG is going to be on by default for pretty much
>> all distro kernels? As such, does it make sense to have this depend on
>> this config symbol?
> 
> If checking the CONFIG means we get back the 1 bit of entropy, I'd say
> keep the check. Some folks will want that bit over P2PDMA.
> 
>> Also +Kees
> 
> Thanks for CCing me! :)
> 

Thanks for review!
Balbir Singh