Re: [PATCH 14/15] PCI/TSM: Create MMIO descriptors via TDISP Report

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] 32-bit unsigned multiplication overflow in MMIO region size calculation silently truncates the size for regions >= 4GB.
- [Medium] Bitwise NOT of a 32-bit `unsigned long` mask zeroes out the upper 32 bits of a 64-bit physical address on 32-bit architectures.
--

commit cfcbb8abea88df9f3a56b20ab3e4fac1bdbf37ea
Author: Dan Williams <djbw@kernel.org>

PCI/TSM: Create MMIO descriptors via TDISP Report

This commit introduces a helper function to create encrypted MMIO descriptors
from a TDISP Report. The descriptors allow the low-level TSM driver to inform
ioremap() how to map the device's MMIO correctly based on the device's
expectations. It also computes the obfuscated base addresses for each
encrypted MMIO range.

Link: https://lore.kernel.org/linux-coco/20251117140007.122062-8-aneesh.kumar@kernel.org/ [1]

> diff --git a/drivers/pci/tsm/core.c b/drivers/pci/tsm/core.c
> --- a/drivers/pci/tsm/core.c
> +++ b/drivers/pci/tsm/core.c

[ ... ]

> +struct pci_tsm_mmio *pci_tsm_mmio_alloc(struct pci_dev *pdev)
> +{

[ ... ]

> +	for (i = 0; i < mmio_range_count; i++) {
> +		u64 range_off;
> +		struct range range;
> +		const struct pci_tsm_devif_mmio *mmio_data = &report->mmio[i];
> +		struct pci_tsm_mmio_entry *entry =
> +			pci_tsm_mmio_entry(mmio, mmio->nr);
> +		u64 tsm_offset = __le64_to_cpu(mmio_data->phys);
> +		u64 size = __le32_to_cpu(mmio_data->nr_pfns) * SZ_4K;

[Severity: High]
Does this calculation suffer from a 32-bit multiplication overflow?

Since __le32_to_cpu() returns a 32-bit unsigned integer and SZ_4K is also
32-bit, if nr_pfns is large enough (representing 4GB or more), the
multiplication will overflow before it is zero-extended and assigned to the
64-bit size variable.

Could this lead to the size being truncated (e.g., yielding 0 for exactly
4GB) and exposing intended secure MMIO space to the VM as unencrypted
memory?

> +		u32 attr = __le32_to_cpu(mmio_data->attributes);
> +		int bar = FIELD_GET(PCI_TSM_DEVIF_REPORT_MMIO_ATTR_RANGE_ID,
> +				    attr);

[ ... ]

> +		if (last_bar < bar) {
> +			unsigned long mask = pci_resource_len(pdev, bar) - 1;
> +
> +			/* Transition to a new bar */
> +			last_bar = bar;
> +
> +			/*
> +			 * Determine the obfuscated base of the BAR. BAR
> +			 * offsets are never obfuscated.
> +			 */
> +			reporting_bar_base = tsm_offset & ~mask;

[Severity: Medium]
Will this bitwise operation truncate the upper 32 bits of the physical
address on 32-bit architectures?

On 32-bit platforms with 64-bit physical addresses (like PAE), unsigned
long is 32 bits. The mask truncates the BAR size, and ~mask flips those
32 bits. When it is promoted to u64 for the bitwise AND with tsm_offset,
the upper 32 bits of the zero-extended ~mask will be 0.

Could this completely erase the upper 32 bits of reporting_bar_base when
compiled for 32-bit platforms?

> +		} else if (tsm_offset < last_reporting_end) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705220819.2472765-1-djbw@kernel.org?part=14