All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lu Baolu <baolu.lu@linux.intel.com>
To: Joerg Roedel <joro@8bytes.org>
Cc: ZhaoJinming <zhaojinming@uniontech.com>,
	Kevin Tian <kevin.tian@intel.com>,
	Dmitry Antipov <dmantipov@yandex.ru>,
	Guanghui Feng <guanghuifeng@linux.alibaba.com>,
	Li RongQing <lirongqing@baidu.com>,
	Desnes Nunes <desnesn@redhat.com>,
	iommu@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: [PATCH 16/20] iommu/vt-d: Fix shift overflow in qi_desc_dev_iotlb_pasid()
Date: Tue,  4 Aug 2026 10:37:10 +0800	[thread overview]
Message-ID: <20260804023714.3080506-17-baolu.lu@linux.intel.com> (raw)
In-Reply-To: <20260804023714.3080506-1-baolu.lu@linux.intel.com>

Callers request a full Device-TLB flush by passing MAX_AGAW_PFN_WIDTH
(64 - VTD_PAGE_SHIFT == 52) as @size_order.  Two shifts in
qi_desc_dev_iotlb_pasid() are not prepared for a value that large:

  unsigned long mask = 1UL << (VTD_PAGE_SHIFT + size_order - 1);
  ...
  if (!IS_ALIGNED(addr, VTD_PAGE_SIZE << size_order))

The first evaluates to 1UL << 63.  On 32-bit builds this is undefined
behaviour; in practice x86 masks the shift count to 5 bits, so the
expression yields 1UL << 31 and ~mask becomes 0x7fffffff.  That value is
zero-extended when it is applied to the 64-bit descriptor, so

  desc->qw1 &= ~mask;

clears qw1[63:32] as well as bit 31.  The ADDR field, which had just been
filled with ones to request the widest possible range, collapses to
0x7ffff000.  As the S bit remains set, hardware decodes the least
significant zero bit of ADDR and invalidates only 2GiB instead of the
entire address space.  Device-TLB entries above that boundary survive the
unmap, leaving an ATS-capable device able to keep accessing memory that
has already been freed.

The second shift, VTD_PAGE_SIZE << size_order, is 1UL << 64 and is
therefore undefined on 64-bit builds too.  On x86_64 the shift count
masks to zero, IS_ALIGNED(addr, 1) is trivially true and the sanity check
silently degrades into a no-op.

Compute both quantities in 64-bit and clamp @size_order to the largest
range the ADDR field can encode.  Capping at 63 - VTD_PAGE_SHIFT keeps
the intended "flush everything" behaviour: qw1[62:12] is set, bit 62 is
cleared as the size indicator and the S bit is set.  The non-PASID
variant qi_desc_dev_iotlb() already uses 1ULL and is unaffected.

Fixes: f701c9f36bcb7 ("iommu/vt-d: Factor out invalidation descriptor composition")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260623060122.3796325-1-guanghuifeng%40linux.alibaba.com
Assisted-by: Claude:claude-opus-5
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
---
 drivers/iommu/intel/iommu.h | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index c00f44db0020..8a59c7c9d0a6 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -1105,12 +1105,20 @@ static inline void qi_desc_dev_iotlb_pasid(u16 sid, u16 pfsid, u32 pasid,
 					   unsigned int size_order,
 					   struct qi_desc *desc)
 {
-	unsigned long mask = 1UL << (VTD_PAGE_SHIFT + size_order - 1);
-
 	desc->qw0 = QI_DEV_EIOTLB_PASID(pasid) | QI_DEV_EIOTLB_SID(sid) |
 		QI_DEV_EIOTLB_QDEP(qdep) | QI_DEIOTLB_TYPE |
 		QI_DEV_IOTLB_PFSID(pfsid);
 
+	/*
+	 * The invalidation range is encoded in the ADDR field, which only
+	 * covers bits 63:12.  Clamp @size_order so that callers asking for a
+	 * full flush (e.g. with MAX_AGAW_PFN_WIDTH) do not overflow the
+	 * shifts below.  The clamped value still spans the whole range that
+	 * the descriptor is able to express.
+	 */
+	if (size_order > 63 - VTD_PAGE_SHIFT)
+		size_order = 63 - VTD_PAGE_SHIFT;
+
 	/*
 	 * If S bit is 0, we only flush a single page. If S bit is set,
 	 * The least significant zero bit indicates the invalidation address
@@ -1120,7 +1128,7 @@ static inline void qi_desc_dev_iotlb_pasid(u16 sid, u16 pfsid, u32 pasid,
 	 * Max Invs Pending (MIP) is set to 0 for now until we have DIT in
 	 * ECAP.
 	 */
-	if (!IS_ALIGNED(addr, VTD_PAGE_SIZE << size_order))
+	if (!IS_ALIGNED(addr, BIT_ULL(VTD_PAGE_SHIFT + size_order)))
 		pr_warn_ratelimited("Invalidate non-aligned address %llx, order %d\n",
 				    addr, size_order);
 
@@ -1136,7 +1144,7 @@ static inline void qi_desc_dev_iotlb_pasid(u16 sid, u16 pfsid, u32 pasid,
 		desc->qw1 |= GENMASK_ULL(size_order + VTD_PAGE_SHIFT - 1,
 					VTD_PAGE_SHIFT);
 		/* Clear size_order bit to indicate size */
-		desc->qw1 &= ~mask;
+		desc->qw1 &= ~BIT_ULL(VTD_PAGE_SHIFT + size_order - 1);
 		/* Set the S bit to indicate flushing more than 1 page */
 		desc->qw1 |= QI_DEV_EIOTLB_SIZE;
 	}
-- 
2.43.0


  parent reply	other threads:[~2026-08-04  2:48 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04  2:36 [PATCH 00/20] [PULL REQUEST] Intel IOMMU updates for v7.3 Lu Baolu
2026-08-04  2:36 ` [PATCH 01/20] iommu/vt-d: Fix UCTP context table slot when copying root entries Lu Baolu
2026-08-04  2:36 ` [PATCH 02/20] iommu/vt-d: Use logical OR operator for privilege mode check Lu Baolu
2026-08-04  2:36 ` [PATCH 03/20] iommu/vt-d: Fix CACHE_TAG_NESTING_DEVTLB polluting shared variables in flush loop Lu Baolu
2026-08-04  3:16   ` 答复: [外部邮件] " Li,Rongqing
2026-08-04  5:28     ` Baolu Lu
2026-08-04  7:18       ` Baolu Lu
2026-08-04  2:36 ` [PATCH 04/20] iommu/vt-d: Use kstrtoint_from_user() in dmar_perf_latency_write() Lu Baolu
2026-08-04  2:36 ` [PATCH 05/20] iommu/vt-d: Fix no_iommu to disable platform opt-in Lu Baolu
2026-08-04  2:37 ` [PATCH 06/20] iommu/vt-d: Force requesting ACS when tboot is enabled Lu Baolu
2026-08-04  2:37 ` [PATCH 07/20] iommu/vt-d: Remove dead code when CONFIG_INTEL_IOMMU is not set Lu Baolu
2026-08-04  2:37 ` [PATCH 08/20] iommu/vt-d: Consolidate dmar policy management and force_on logic Lu Baolu
2026-08-04  2:37 ` [PATCH 09/20] iommu/vt-d: Use dmar_can_force_on() for platform opt-in Lu Baolu
2026-08-04  2:37 ` [PATCH 10/20] iommu/vt-d: Call dmar_can_force_on() for tboot opt-in Lu Baolu
2026-08-04  2:37 ` [PATCH 11/20] iommu/vt-d: Remove the 'force_on' variable Lu Baolu
2026-08-04  2:37 ` [PATCH 12/20] iommu/vt-d: Remove dmar_disabled Lu Baolu
2026-08-04  2:37 ` [PATCH 13/20] iommu/vt-d: Support the new DMA_REMAP_OPT_OUT flag bit Lu Baolu
2026-08-04  2:37 ` [PATCH 14/20] iommu/vt-d: Cache max domain ID to avoid redundant calculation Lu Baolu
2026-08-04  2:37 ` [PATCH 15/20] iommu/vt-d: Fix copied_tables bitmap leak on error in copy_translation_tables Lu Baolu
2026-08-04  2:37 ` Lu Baolu [this message]
2026-08-04  5:54   ` [PATCH 16/20] iommu/vt-d: Fix shift overflow in qi_desc_dev_iotlb_pasid() Baolu Lu
2026-08-04  2:37 ` [PATCH 17/20] iommu/vt-d: Clear Present bit before tearing down copied context entry Lu Baolu
2026-08-04  2:37 ` [PATCH 18/20] iommu/vt-d: Fix iopf_refcount leak on RID domain replacement Lu Baolu
2026-08-04  2:37 ` [PATCH 19/20] iommu/vt-d: Tear down scalable-mode context on probe failure Lu Baolu
2026-08-04  2:37 ` [PATCH 20/20] iommu/vt-d: Flush context cache with correct SID when tearing down aliases Lu Baolu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260804023714.3080506-17-baolu.lu@linux.intel.com \
    --to=baolu.lu@linux.intel.com \
    --cc=desnesn@redhat.com \
    --cc=dmantipov@yandex.ru \
    --cc=guanghuifeng@linux.alibaba.com \
    --cc=iommu@lists.linux.dev \
    --cc=joro@8bytes.org \
    --cc=kevin.tian@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lirongqing@baidu.com \
    --cc=zhaojinming@uniontech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.