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.

Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Link: https://lore.kernel.org/all/1462225276-106993-1-git-send-email-thgarnie@google.com/


Signed-off-by: Balbir Singh <balbirs@nvidia.com>
---

Testing:

  commit 0483e1fa6e09d ("x86/mm: Implement ASLR for kernel memory regions") mentions the
  problems that the following problems need to be addressed.

  1 The three target memory sections are never at the same place between
    boots.
  2 The physical memory mapping can use a virtual address not aligned on
    the PGD page table.
  3 Have good entropy early at boot before get_random_bytes is available.
  4 Add optional padding for memory hotplug compatibility.

  Ran an automated test to ensure that (1) holds true across several
  iterations of automated reboot testing. 2, 3 and 4 are not impacted
  by this patch.

  Manual Testing on a system where the problem reproduces
  
  1. With KASLR

     Hotplug memory [0x240000000000-0x242000000000] exceeds maximum addressable range [0x0-0xaffffffffff]
     ------------[ cut here ]------------
  2. With the fixes

     added peer-to-peer DMA memory 0x240000000000-0x241fffffffff

     KASLR is still enabled as seen by kaslr_offset() (difference
     between __START_KERNEL and _stext)
  3. Without the fixes and KASLR disabled

     added peer-to-peer DMA memory 0x240000000000-0x241fffffffff

     KASLR is disabled, kaslr_offset() is 0.

On my system with 46 bits physical address and 4 level page tables (no
LA57), the remaining entropy dropped from 49 TiB to 20 TiB

 arch/x86/mm/kaslr.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
index 11a93542d198..3c306de52fd4 100644
--- a/arch/x86/mm/kaslr.c
+++ b/arch/x86/mm/kaslr.c
@@ -113,8 +113,14 @@ void __init kernel_randomize_memory(void)
 	memory_tb = DIV_ROUND_UP(max_pfn << PAGE_SHIFT, 1UL << TB_SHIFT) +
 		CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
 
-	/* Adapt physical memory region size based on available memory */
-	if (memory_tb < kaslr_regions[0].size_tb)
+	/*
+	 * Adapt physical memory region size based on available memory,
+	 * except when CONFIG_PCI_P2PDMA is enabled. P2PDMA exposes the
+	 * device BAR space assuming the direct map space is large enough
+	 * for creating a ZONE_DEVICE mapping in the direct map corresponding
+	 * to the physical BAR address.
+	 */
+	if (!IS_ENABLED(CONFIG_PCI_P2PDMA) && (memory_tb < kaslr_regions[0].size_tb))
 		kaslr_regions[0].size_tb = memory_tb;
 
 	/*
-- 
2.48.0

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.

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?

Also +Kees

> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Link: https://lore.kernel.org/all/1462225276-106993-1-git-send-email-thgarnie@google.com/
> 
> 
> Signed-off-by: Balbir Singh <balbirs@nvidia.com>
> ---
> 
> Testing:
> 
>   commit 0483e1fa6e09d ("x86/mm: Implement ASLR for kernel memory regions") mentions the
>   problems that the following problems need to be addressed.
> 
>   1 The three target memory sections are never at the same place between
>     boots.
>   2 The physical memory mapping can use a virtual address not aligned on
>     the PGD page table.
>   3 Have good entropy early at boot before get_random_bytes is available.
>   4 Add optional padding for memory hotplug compatibility.
> 
>   Ran an automated test to ensure that (1) holds true across several
>   iterations of automated reboot testing. 2, 3 and 4 are not impacted
>   by this patch.
> 
>   Manual Testing on a system where the problem reproduces
>   
>   1. With KASLR
> 
>      Hotplug memory [0x240000000000-0x242000000000] exceeds maximum addressable range [0x0-0xaffffffffff]
>      ------------[ cut here ]------------
>   2. With the fixes
> 
>      added peer-to-peer DMA memory 0x240000000000-0x241fffffffff
> 
>      KASLR is still enabled as seen by kaslr_offset() (difference
>      between __START_KERNEL and _stext)
>   3. Without the fixes and KASLR disabled
> 
>      added peer-to-peer DMA memory 0x240000000000-0x241fffffffff
> 
>      KASLR is disabled, kaslr_offset() is 0.
> 
> On my system with 46 bits physical address and 4 level page tables (no
> LA57), the remaining entropy dropped from 49 TiB to 20 TiB
> 
>  arch/x86/mm/kaslr.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/kaslr.c b/arch/x86/mm/kaslr.c
> index 11a93542d198..3c306de52fd4 100644
> --- a/arch/x86/mm/kaslr.c
> +++ b/arch/x86/mm/kaslr.c
> @@ -113,8 +113,14 @@ void __init kernel_randomize_memory(void)
>  	memory_tb = DIV_ROUND_UP(max_pfn << PAGE_SHIFT, 1UL << TB_SHIFT) +
>  		CONFIG_RANDOMIZE_MEMORY_PHYSICAL_PADDING;
>  
> -	/* Adapt physical memory region size based on available memory */
> -	if (memory_tb < kaslr_regions[0].size_tb)
> +	/*
> +	 * Adapt physical memory region size based on available memory,
> +	 * except when CONFIG_PCI_P2PDMA is enabled. P2PDMA exposes the
> +	 * device BAR space assuming the direct map space is large enough
> +	 * for creating a ZONE_DEVICE mapping in the direct map corresponding
> +	 * to the physical BAR address.
> +	 */
> +	if (!IS_ENABLED(CONFIG_PCI_P2PDMA) && (memory_tb < kaslr_regions[0].size_tb))
>  		kaslr_regions[0].size_tb = memory_tb;
>  
>  	/*
> -- 
> 2.48.0
>

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. :)

> 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! :)

-Kees

-- 
Kees Cook

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

On Fri, Feb 07, 2025 at 08:22:23AM +1100, Balbir Singh wrote:
> 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.

Actually, it might be more useful to note it in the CONFIG help
text? "This may reduce direct map ASLR entropy by 1 bit with 46 physical
bits, X bits with YY physical bits, [etc...]"

-Kees

-- 
Kees Cook

On 2/7/25 08:46, Kees Cook wrote:
> On Fri, Feb 07, 2025 at 08:22:23AM +1100, Balbir Singh wrote:
>> 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.
> 
> Actually, it might be more useful to note it in the CONFIG help
> text? "This may reduce direct map ASLR entropy by 1 bit with 46 physical
> bits, X bits with YY physical bits, [etc...]"
> 

I worded it as follows in drivers/pci/Kconfig

	  Enabling this option will reduce the entropy of x86 KASLR memory
	  regions. For example - on a 46 bit system, the entropy goes down
	  from 16 bits to 15 bits. The actual reduction in entropy depends
	  on the physical address bits, on processor features, kernel config
	  (5 level page table) and physical memory present on the system.

Does that seem reasonable?

Balbir

On Fri, Feb 07, 2025 at 09:23:50AM +1100, Balbir Singh wrote:
> On 2/7/25 08:46, Kees Cook wrote:
> > On Fri, Feb 07, 2025 at 08:22:23AM +1100, Balbir Singh wrote:
> >> 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.
> > 
> > Actually, it might be more useful to note it in the CONFIG help
> > text? "This may reduce direct map ASLR entropy by 1 bit with 46 physical
> > bits, X bits with YY physical bits, [etc...]"
> > 
> 
> I worded it as follows in drivers/pci/Kconfig
> 
> 	  Enabling this option will reduce the entropy of x86 KASLR memory
> 	  regions. For example - on a 46 bit system, the entropy goes down
> 	  from 16 bits to 15 bits. The actual reduction in entropy depends
> 	  on the physical address bits, on processor features, kernel config
> 	  (5 level page table) and physical memory present on the system.
> 
> Does that seem reasonable?

Yeah, I like it. Thanks!

-- 
Kees Cook