linux-kernel - RE: [PATCH 1/1] iommu/vt-d: Fix aligned pages in calculate_psi_aligned

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: <BN9PR11MB527613512AB7E8D3B4B8FEBD8CDB2@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Tue, 9 Jul 2024 06:58:50 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Baolu Lu <baolu.lu@...ux.intel.com>, Joerg Roedel <joro@...tes.org>, "Will
 Deacon" <will@...nel.org>, Robin Murphy <robin.murphy@....com>, Louis Maliyam
	<louispm@...gle.com>
CC: "iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/1] iommu/vt-d: Fix aligned pages in
 calculate_psi_aligned_address()

> From: Baolu Lu <baolu.lu@...ux.intel.com>
> Sent: Tuesday, July 9, 2024 10:54 AM
> 
> On 7/8/24 8:14 PM, Lu Baolu wrote:
> > The helper calculate_psi_aligned_address() is used to convert an arbitrary
> > range into a size-aligned one.
> >
> > The aligned_pages variable is calculated from input start and end, but is
> > not adjusted when the start pfn is not aligned and the mask is adjusted,
> > which results in an incorrect number of pages returned.
> >
> > The number of pages is used by qi_flush_piotlb() to flush caches for the
> > first-stage translation. With the wrong number of pages, the cache is not
> > synchronized, leading to inconsistencies in some cases.
> >
> > Fixes: c4d27ffaa8eb ("iommu/vt-d: Add cache tag invalidation helpers")
> > Signed-off-by: Lu Baolu<baolu.lu@...ux.intel.com>
> > ---
> >   drivers/iommu/intel/cache.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
> > index e8418cdd8331..113834742107 100644
> > --- a/drivers/iommu/intel/cache.c
> > +++ b/drivers/iommu/intel/cache.c
> > @@ -246,6 +246,7 @@ static unsigned long
> calculate_psi_aligned_address(unsigned long start,
> >   		 */
> >   		shared_bits = ~(pfn ^ end_pfn) & ~bitmask;
> >   		mask = shared_bits ? __ffs(shared_bits) : BITS_PER_LONG;
> > +		aligned_pages = 1UL << mask;
> 
> Hmm, it appears that if mask is equal to BITS_PER_LONG (which is
> typically 64), the left shift operation will overflow.
> 
> So perhaps we need another line of change:
> 
> diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
> index 113834742107..44e92638c0cd 100644
> --- a/drivers/iommu/intel/cache.c
> +++ b/drivers/iommu/intel/cache.c
> @@ -245,7 +245,7 @@ static unsigned long
> calculate_psi_aligned_address(unsigned long start,
>                   * shared_bits are all equal in both pfn and end_pfn.
>                   */
>                  shared_bits = ~(pfn ^ end_pfn) & ~bitmask;
> -               mask = shared_bits ? __ffs(shared_bits) : BITS_PER_LONG;
> +               mask = shared_bits ? __ffs(shared_bits) :
> MAX_AGAW_PFN_WIDTH;
>                  aligned_pages = 1UL << mask;
>          }
> 
> I will make above another fix as it already causes overflow in another
> path.
> 
> Kevin, sound good to you?
> 

yes. for both:

Reviewed-by: Kevin Tian <kevin.tian@...el.com>