linux-kernel - Re: [PATCH] perf/x86/intel/pt: Update topa_entry base len to support 52-bit physical addresses

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]

Message-ID: <6ab8c7fd-c718-49ff-bbbb-9241293127f7@intel.com>
Date: Wed, 19 Jun 2024 13:00:34 +0300
From: Adrian Hunter <adrian.hunter@...el.com>
To: Dave Hansen <dave.hansen@...el.com>,
 Marco Cavenati <cavenati.marco@...il.com>, linux-perf-users@...r.kernel.org,
 linux-kernel@...r.kernel.org, alexander.shishkin@...ux.intel.com
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
 namhyung@...nel.org, mark.rutland@....com, jolsa@...nel.org,
 irogers@...gle.com, kan.liang@...ux.intel.com, tglx@...utronix.de,
 bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com
Subject: Re: [PATCH] perf/x86/intel/pt: Update topa_entry base len to support
 52-bit physical addresses

On 18/06/24 20:59, Dave Hansen wrote:
> On 6/18/24 04:06, Marco Cavenati wrote:
>> Increase topa_entry base to 40 bits to accommodate page addresses in
>> systems with 52-bit physical addresses.
>> The Base Physical Address field (base) has a length of MAXPHYADDR - 12 as
>> stated in Intel's SDM chapter 33.2.7.2.
>> The maximum MAXPHYADDR is 52 as stated in SDM 4.1.4.
>> Therefore, the maximum base bit length is 40.
> 
> This makes it sound like it's _adding_ support for larger physical
> addresses.  It really was a bug from day one.  MAXPHYADDR has been
> defined to be "at most 52" for a long, long time.  I think it was well
> before 5-level paging came on the scene and actual MAXPHYADDR=52 systems
> came along.
> 
> It probably needs to say something more along the lines of:
> 
> 	topa_entry->base needs to store a pfn.  It obviously needs to be
> 	large enough to store the largest possible x86 pfn which is
> 	MAXPHYADDR-PAGE_SIZE (52-12).  So it is 4 bits too small.
> 
> This isn't the only bug in the area:
> 
>> static void *pt_buffer_region(struct pt_buffer *buf)
>> {
>>         return phys_to_virt(TOPA_ENTRY(buf->cur, buf->cur_idx)->base << TOPA_SHIFT);
>> }
> 
> At this point, ->base is still a 40-bit (or 36-bit before this patch)
> type.  If it has anything in the high 12 bits, a <<TOPA_SHIFT will just
> lose those bits.

Yes

> 
> But maybe I'm reading it wrong.  If I'm right, this malfunctions at pfns
> over 36-12=24 bits, or 64GB of RAM.  Is it possible nobody has ever
> allocated a 'struct pt_buffer' over 64GB?  Or is this somehow tolerant
> of reading garbage?

Yes, it will go wrong with any physical address above 64GB - 1.
i.e. the machine just needs more than 64GB of memory.

However that code is used only in one place which is conditional on
!intel_pt_validate_hw_cap(PT_CAP_topa_multiple_entries) which is true
only for Broadwell.  Also "snapshot" (and sampling) modes are
unaffected.

Testing on a Broadwell with 400GB of memory confirmed the issue.