From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1901142E8F1 for ; Mon, 3 Aug 2026 18:28:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785781742; cv=none; b=hVZ1+40P+5jo6sh9G+lYNph1jupyCemWEwLCzjgENzdeUJ0BvVujif88+DkyNpcFqC3goAHP3oWDmiNUMVlqZiSvrVWkXoA2AAbVRW5m8hdiNLPNVpxA7EagPz0cTIw0dKzxFdFWM5bm7Eg2tVfdZc6G0Ogl1uEQnqIYd12NAK0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785781742; c=relaxed/simple; bh=5dAW2wmbtidI5gVpvg/cVrzbBBJdhnVmI6fLJTdKrrY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=nQtrubZfMPt0ddjDO5A/kTnPRQ4Tzm2IjEuGWta7TiYhEWFbYaX0t/WyZCu5AputwjT1awsrDJVDk0x01NuvlRHj0yfy3RYH5K1+hGKyN1EJ2bI804e1YcLockV0obKWA20bVDCA5ofYsNakP5WQVCTHvGpbIDN60JWgDiLT6r0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=iSyFt5p2; arc=none smtp.client-ip=209.85.214.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="iSyFt5p2" Received: by mail-pl1-f172.google.com with SMTP id d9443c01a7336-2cab97c86bdso23065ad.1 for ; Mon, 03 Aug 2026 11:28:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1785781739; x=1786386539; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=YCj2r/JqY5u5nyX++s6hFCTol5l9mZg/fkApmxyULqA=; b=iSyFt5p2kaRZat/YaTCZ3s3maqt36oI9W4N4+KXf4BV30Oahsu/TnAVcgWJSdruPcX QWnYtb76Bg89o4jn3D9e06to6DS+obFkxmqhfaHfh2XN0NuyGzlJYmuJKgCB7eQ5xG1v Sj99EH4PhogTZLq5DSl9nRdVZQ2ntPqd5xKNyxUpZ2alwT6H/FiXnpyHlvGI+xR/ISkJ 4dFv04bea7h9U6iYhbfEE/gPSqkD1iiBQYrH+GLCvUVDX6PukUAN0sucQlpMmUyCiAM9 jV3hAk+KitZCZQKiIONsmz98Po8LWC3lbKL36rwbEs2sJSbrQ0CbX0VttJqEzWehLDf7 ZDCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785781739; x=1786386539; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=YCj2r/JqY5u5nyX++s6hFCTol5l9mZg/fkApmxyULqA=; b=fvwd26MGVALelbkTcsLyIStt9UnCd1lqV2aTTyy0CM/EXMOyauc4/YqgmfXCwrKerI 6ZvE93jEua3MBor/L/M05Hfmxs1JSZocOdieYX/dh8zVxXCKyG0U4QgpXAYevaqdrXFJ B+37EJ/mKUm/6J8a6FFBJPUCIaJDz+T6mQ5jmxhXhj7jebmiEfnc6HRe1sDOed1WBaNf kmsWKyByonmfQGeZnrhz3yEkFVulBQEjKFHOut314KYH2FzjTL+RyOM9WjkNZ7TkrsXU DDaEHHYSmJPp4/632gj2i98HxC9QBnMJ+9t6HSdgv6aCNyL5U+zeFGENyXcA7V3T1bxg UYAA== X-Forwarded-Encrypted: i=1; AHgh+Rriay18BgTk1A8pGyt8+eae9glnuBcBSQXm7ldPTJ2UJrRVyI7sDO4Apax2VEY4unu1wn5+9nqvNuYdU60=@vger.kernel.org X-Gm-Message-State: AOJu0YziM7oZroZcAwYsHJUJNzDQcgSizJyQf0d8yeHtoBY44tLg4qbz 7wyv3jzelGtQpnzJ92gh3+sB3/LpZ0fQdKjhhIND27czl+tSzOM+5F+fIZ7rFDFxNQ== X-Gm-Gg: AR+sD10Ed6ICdIdJBU2aWyjsrg0BE1CxbXzp+mPligLLgUDF04P9TENK+CZQpOZ4UFZ MJfE4GAycx9qEeppEZ1fLkve5DqViQ1CJaEXgpBrGk3iqrw5aWdQ8Y2RQPL8DWOpY8SxrRnMCIL 5XJP6L5QES21+cRsfLn9ETcbdmf9GxMi5jG5fLQ1bmXsaBZkEmTbg2d8KSnD3bbMIb1G+Z/fR49 kCW5DckrP8ODrwJhnhD3vpEIZGwDA93HRbZhPne+QsrQUYavOhkoSVtTNdwCMUAcoiBXq8smb4x Q9B8G4EcV+bfbbh0dZnEzjriy7aa16GmkpzGf4ex50wtAldHkXsEKw4GzdVXRhY9nwrwsvtj10y Z/5RS/Z4gtYOjZUNKLuPOOvs29r0p96SLLYF+ZSM0qRhD42r/jlLMgFzKLzWOpmhRW1Kn5CNsxL NtF2AezyRvab0s2bxkrxSZoFZbHgy1nt6RC7LXlp7j7wXFXl4amkLzAR0G1egwlti7FkoxE4B4n PQGbrBZp0I2v2PKUamhVhxlA6hnx23gvn4c12pO4dRJHkQLB14a0HzoF+kYiW5BO+NhiA== X-Received: by 2002:a17:902:ea0b:b0:2ce:b436:272a with SMTP id d9443c01a7336-2d08cdb06a7mr964365ad.3.1785781738650; Mon, 03 Aug 2026 11:28:58 -0700 (PDT) Received: from google.com (210.87.127.34.bc.googleusercontent.com. [34.127.87.210]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-cbe396ea598sm4113578a12.19.2026.08.03.11.28.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 03 Aug 2026 11:28:58 -0700 (PDT) Date: Mon, 3 Aug 2026 18:28:55 +0000 From: Samiullah Khawaja To: Lu Baolu Cc: Joerg Roedel , Will Deacon , Robin Murphy , Jason Gunthorpe , Kevin Tian , iommu@lists.linux.dev, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Sashiko Subject: Re: [PATCH 1/5] iommu/vt-d: Fix shift overflow in qi_desc_dev_iotlb_pasid() Message-ID: References: <20260731054329.2948252-1-baolu.lu@linux.intel.com> <20260731054329.2948252-2-baolu.lu@linux.intel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20260731054329.2948252-2-baolu.lu@linux.intel.com> On Fri, Jul 31, 2026 at 01:43:25PM +0800, Lu Baolu wrote: >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 >Closes: https://sashiko.dev/#/patchset/20260623060122.3796325-1-guanghuifeng%40linux.alibaba.com >Assisted-by: Claude:claude-opus-5 >Signed-off-by: Lu Baolu >--- > 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 > > Reviewed-by: Samiullah Khawaja