linux-kernel - Re: [RFC PATCH v1 34/38] coco: guest: arm64: Validate mmio range found in the interface report

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: <3a323046-3f67-4e1f-903d-4ce090059cad@nvidia.com>
Date: Mon, 4 Aug 2025 11:27:38 +0300
From: Arto Merilainen <amerilainen@...dia.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@...nel.org>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, aik@....com,
 lukas@...ner.de, Samuel Ortiz <sameo@...osinc.com>,
 Xu Yilun <yilun.xu@...ux.intel.com>, Jason Gunthorpe <jgg@...pe.ca>,
 Suzuki K Poulose <Suzuki.Poulose@....com>,
 Steven Price <steven.price@....com>,
 Catalin Marinas <catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>,
 Will Deacon <will@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>,
 kvmarm@...ts.linux.dev, linux-coco@...ts.linux.dev
Subject: Re: [RFC PATCH v1 34/38] coco: guest: arm64: Validate mmio range
 found in the interface report


On 4.8.2025 9.37, Aneesh Kumar K.V wrote:
> Arto Merilainen <amerilainen@...dia.com> writes:
> 
>> On 28.7.2025 16.52, Aneesh Kumar K.V (Arm) wrote:
>>
>>> +    for (int i = 0; i < interface_report->mmio_range_count; i++, mmio_range++) {
>>> +
>>> +            /*FIXME!! units in 4K size*/
>>> +            range_id = FIELD_GET(TSM_INTF_REPORT_MMIO_RANGE_ID, mmio_range->range_attributes);
>>> +
>>> +            /* no secure interrupts */
>>> +            if (msix_tbl_bar != -1 && range_id == msix_tbl_bar) {
>>> +                    pr_info("Skipping misx table\n");
>>> +                    continue;
>>> +            }
>>> +
>>> +            if (msix_pba_bar != -1 && range_id == msix_pba_bar) {
>>> +                    pr_info("Skipping misx pba\n");
>>> +                    continue;
>>> +            }
>>> +
>>
>>
>> MSI-X and PBA can be placed to a BAR that has other registers as well.
>> While the PCIe specification recommends BAR-level isolation for MSI-X
>> structures, it is not mandated. It is enough to have sufficient
>> isolation within the BAR. Therefore, skipping the MSI-X and PBA BARs
>> altogether may leave registers unintentionally mapped via unprotected
>> IPA when they should have been mapped via protected IPA.
>>
>> Instead of skipping the whole BAR, would it make sense to determine
>> where the MSI-X related regions reside, and skip validation only from
>> these regions?
>>
> 
> Yes, that was added because at one point the FVP model was including the
> MSI-X table and PBA regions in the interface report.

The issue I am raising is a different one. The MSI-X table and PBA may 
reside in the middle of a BAR range, and the BAR range may contain 
registers which should be accessible only via protected IPA. In this 
case I would expect the pages before and after the MSI-X table (and PBA) 
to be validated while the pages related to MSI-X structures would be 
left unprotected.

Given that the MSI-X table or PBA regions shouldn't be present in the 
interface report, the code needs to first find out where the MSI-X 
structures reside, and use this information to determine which 
"sub-ranges" should be skipped over during validation.
> If I understand correctly, we shouldn't expect to see those regions in
> the report unless secure interrupts are supported. The BAR-based
> skipping was added as a workaround to handle the FVP issue.
> 

Correct. Assuming that RMM doesn't lock the MSI-X table, I'd expect 
these regions to be omitted in the interface report.

> I believe we can drop that workaround now—if those regions are
> incorrectly present, the below validation logic should catch and
> reject them appropriately. Does that sound reasonable?
> 
>                  /* No secure interrupts, we should not find this set, ignore for now. */
>                  if (FIELD_GET(TSM_INTF_REPORT_MMIO_MSIX_TABLE, mmio_range->range_attributes) ||
>                      FIELD_GET(TSM_INTF_REPORT_MMIO_PBA, mmio_range->range_attributes)) {
>                          pci_info(pdev, "Skipping MSIX (%ld/%ld) range [%d] #%d %d pages, %llx..%llx\n",
>                                   FIELD_GET(TSM_INTF_REPORT_MMIO_MSIX_TABLE, mmio_range->range_attributes),
>                                   FIELD_GET(TSM_INTF_REPORT_MMIO_PBA, mmio_range->range_attributes),
>                                   i, range_id, mmio_range->num_pages, r->start, r->end);
>                          continue;
>                  }
> 
> 

First, I think this is skipping over the whole range => if there are 
registers outside of the MSI-X structures (but within the same BAR), 
they won't be validated.

Second, if the MSI-X regions are present in the interface report, 
wouldn't it - in the common case - mean that the device expects the 
ranges to be accessed with T=1? If this happens unexpectedly, it sounds 
that MSI-X wouldn't be usable. I wonder if the code should simply return 
an error instead if informing the user via pci_info()...

- R2