Re: [Intel-gfx] [PATCH] iommu/intel: Handle 36b addressing for x86-32

From: Lu Baolu <baolu.lu@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	iommu@lists.linux-foundation.org
Cc: James Sewart <jamessewart@arista.com>,
	intel-gfx@lists.freedesktop.org, Joerg Roedel <jroedel@suse.de>,
	stable@vger.kernel.org, baolu.lu@linux.intel.com
Subject: Re: [Intel-gfx] [PATCH] iommu/intel: Handle 36b addressing for x86-32
Date: Tue, 25 Aug 2020 11:13:31 +0800	[thread overview]
Message-ID: <98c34926-6085-81f2-dce2-53ef1ce1edc8@linux.intel.com> (raw)
In-Reply-To: <20200822160209.28512-1-chris@chris-wilson.co.uk>

Hi Chris,

On 2020/8/23 0:02, Chris Wilson wrote:
> Beware that the address size for x86-32 may exceed unsigned long.
> 
> [    0.368971] UBSAN: shift-out-of-bounds in drivers/iommu/intel/iommu.c:128:14
> [    0.369055] shift exponent 36 is too large for 32-bit type 'long unsigned int'
> 
> If we don't handle the wide addresses, the pages are mismapped and the
> device read/writes go astray, detected as DMAR faults and leading to
> device failure. The behaviour changed (from working to broken) in commit
> fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer"), but
commit <fa954e683178> ("iommu/vt-d: Delegate the dma domain to upper layer")

and adjust the title as "iommu/vt-d: Handle 36bit addressing for x86-32"

with above two changes,

Acked-by: Lu Baolu <baolu.lu@linux.intel.com>

Best regards,
baolu

> the error looks older.
> 
> Fixes: fa954e683178 ("iommu/vt-d: Delegate the dma domain to upper layer")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: James Sewart <jamessewart@arista.com>
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Cc: Joerg Roedel <jroedel@suse.de>
> Cc: <stable@vger.kernel.org> # v5.3+
> ---
>   drivers/iommu/intel/iommu.c | 14 +++++++-------
>   1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 2e9c8c3d0da4..ba78a2e854f9 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -123,29 +123,29 @@ static inline unsigned int level_to_offset_bits(int level)
>   	return (level - 1) * LEVEL_STRIDE;
>   }
>   
> -static inline int pfn_level_offset(unsigned long pfn, int level)
> +static inline int pfn_level_offset(u64 pfn, int level)
>   {
>   	return (pfn >> level_to_offset_bits(level)) & LEVEL_MASK;
>   }
>   
> -static inline unsigned long level_mask(int level)
> +static inline u64 level_mask(int level)
>   {
> -	return -1UL << level_to_offset_bits(level);
> +	return -1ULL << level_to_offset_bits(level);
>   }
>   
> -static inline unsigned long level_size(int level)
> +static inline u64 level_size(int level)
>   {
> -	return 1UL << level_to_offset_bits(level);
> +	return 1ULL << level_to_offset_bits(level);
>   }
>   
> -static inline unsigned long align_to_level(unsigned long pfn, int level)
> +static inline u64 align_to_level(u64 pfn, int level)
>   {
>   	return (pfn + level_size(level) - 1) & level_mask(level);
>   }
>   
>   static inline unsigned long lvl_to_nr_pages(unsigned int lvl)
>   {
> -	return  1 << min_t(int, (lvl - 1) * LEVEL_STRIDE, MAX_AGAW_PFN_WIDTH);
> +	return 1UL << min_t(int, (lvl - 1) * LEVEL_STRIDE, MAX_AGAW_PFN_WIDTH);
>   }
>   
>   /* VT-d pages must always be _smaller_ than MM pages. Otherwise things
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx