Re: [PATCH v2 18/30] iommu/s390: implement iova_to_phys_length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing Intra-Page Offset in Physical Address Translation
- [High] Null Pointer Dereference in get_rto_from_iova()
--

commit f8957d220cfee6484b5b88e6614c133bcac327ca
Author: Guanghui Feng <guanghuifeng@linux.alibaba.com>

iommu/s390: implement iova_to_phys_length
    
Implement iova_to_phys_length for s390 IOMMU driver,
returning the actual PTE mapping size.

> diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c
> index f148f559ac564..b12ed389e1bd9 100644
> --- a/drivers/iommu/s390-iommu.c
> +++ b/drivers/iommu/s390-iommu.c
> @@ -986,22 +986,26 @@ static unsigned long *get_rto_from_iova(struct s390_domain *domain,
[ ... ]
> -static phys_addr_t s390_iommu_iova_to_phys(struct iommu_domain *domain,
> -					   dma_addr_t iova)
> +static phys_addr_t s390_iommu_iova_to_phys_length(struct iommu_domain *domain,
> +						  dma_addr_t iova,
> +						  size_t *mapped_length)
>  {
[ ... ]
>  	rto = get_rto_from_iova(s390_domain, iova);

[Severity: High]
This is a pre-existing issue, but does get_rto_from_iova() safely handle
unmapped IOVAs?

Looking at get_rto_from_iova() in drivers/iommu/s390-iommu.c, it calls
get_rso_from_iova() which can return NULL if the top-level Region First entry
is invalid. However, the pointer is dereferenced unconditionally:

drivers/iommu/s390-iommu.c:get_rto_from_iova() {
	...
	case ZPCI_TABLE_TYPE_RFX:
	case ZPCI_TABLE_TYPE_RSX:
		rso = get_rso_from_iova(domain, iova);
		rsx = calc_rsx(iova);
		rse = READ_ONCE(rso[rsx]);
	...
}

Would calling iommu_iova_to_phys() on an unmapped IOVA trigger a NULL pointer
dereference here?

>  	if (!rto)
> -		return 0;
> +		return PHYS_ADDR_MAX;
>  
>  	rtx = calc_rtx(iova);
>  	sx = calc_sx(iova);
> @@ -1014,8 +1018,11 @@ static phys_addr_t s390_iommu_iova_to_phys(struct iommu_domain *domain,
>  		if (reg_entry_isvalid(ste)) {
>  			pto = get_st_pto(ste);
>  			pte = READ_ONCE(pto[px]);
> -			if (pt_entry_isvalid(pte))
> +			if (pt_entry_isvalid(pte)) {
>  				phys = pte & ZPCI_PTE_ADDR_MASK;

[Severity: High]
This isn't a bug introduced by this patch, but does this physical address
translation truncate the intra-page offset?

The returned physical address is constructed by masking the page table entry
with ZPCI_PTE_ADDR_MASK, but it doesn't incorporate the sub-page offset from
the original IOVA. Since the IOMMU core API iommu_iova_to_phys() expects the
exact physical address corresponding to the byte-level IOVA, could truncating
the offset cause callers querying unaligned IOVAs to receive incorrect
addresses?

> +				if (mapped_length)
> +					*mapped_length = SZ_4K;
> +			}
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602104637.1219810-1-guanghuifeng@linux.alibaba.com?part=18