linux-kernel - RE: [PATCH v2] iommu/vt-d: Fix potential lockup if qi_submit

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <BN9PR11MB5276FD0AB21F4702175185D98CB12@BN9PR11MB5276.namprd11.prod.outlook.com>
Date: Wed, 31 Jul 2024 05:58:25 +0000
From: "Tian, Kevin" <kevin.tian@...el.com>
To: Jacob Pan <jacob.jun.pan@...ux.intel.com>, "iommu@...ts.linux.dev"
	<iommu@...ts.linux.dev>, LKML <linux-kernel@...r.kernel.org>, Lu Baolu
	<baolu.lu@...ux.intel.com>
CC: "Liu, Yi L" <yi.l.liu@...el.com>, "Zhang, Tina" <tina.zhang@...el.com>,
	"Kumar, Sanjay K" <sanjay.k.kumar@...el.com>
Subject: RE: [PATCH v2] iommu/vt-d: Fix potential lockup if qi_submit_sync
 called with 0 count

> From: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> Sent: Monday, July 29, 2024 5:01 AM
> 
> From: Sanjay K Kumar <sanjay.k.kumar@...el.com>
> 
> If qi_submit_sync() is invoked with 0 invalidation descriptors (for
> instance, for DMA draining purposes), we can run into a bug where a
> submitting thread fails to detect the completion of invalidation_wait.
> Subsequently, this led to a soft lockup. Currently, there is no impact
> by this bug on the existing users because no callers are submitting
> invalidations with 0 descriptors. This fix will enable future users
> (such as DMA drain) calling qi_submit_sync() with 0 count.
> 
> Suppose thread T1 invokes qi_submit_sync() with non-zero descriptors,
> while
> concurrently, thread T2 calls qi_submit_sync() with zero descriptors. Both
> threads then enter a while loop, waiting for their respective descriptors
> to complete. T1 detects its completion (i.e., T1's invalidation_wait status
> changes to QI_DONE by HW) and proceeds to call reclaim_free_desc() to
> reclaim all descriptors, potentially including adjacent ones of other
> threads that are also marked as QI_DONE.
> 
> During this time, while T2 is waiting to acquire the qi->q_lock, the IOMMU
> hardware may complete the invalidation for T2, setting its status to
> QI_DONE. However, if T1's execution of reclaim_free_desc() frees T2's
> invalidation_wait descriptor and changes its status to QI_FREE, T2 will
> not observe the QI_DONE status for its invalidation_wait and will
> indefinitely remain stuck.
> 
> This soft lockup does not occur when only non-zero descriptors are
> submitted.In such cases, invalidation descriptors are interspersed among
> wait descriptors with the status QI_IN_USE, acting as barriers. These
> barriers prevent the reclaim code from mistakenly freeing descriptors
> belonging to other submitters.
> 
> Considered the following example timeline:
> 	T1			T2
> ========================================
> 	ID1
> 	WD1
> 	while(WD1!=QI_DONE)
> 	unlock
> 				lock
> 	WD1=QI_DONE*		WD2
> 				while(WD2!=QI_DONE)
> 				unlock
> 	lock
> 	WD1==QI_DONE?
> 	ID1=QI_DONE		WD2=DONE*
> 	reclaim()
> 	ID1=FREE
> 	WD1=FREE
> 	WD2=FREE
> 	unlock
> 				soft lockup! T2 never sees QI_DONE in WD2
> 
> Where:
> ID = invalidation descriptor
> WD = wait descriptor
> * Written by hardware
> 
> The root of the problem is that the descriptor status QI_DONE flag is used
> for two conflicting purposes:
> 1. signal a descriptor is ready for reclaim (to be freed)
> 2. signal by the hardware that a wait descriptor is complete
> 
> The solution (in this patch) is state separation by using QI_FREE flag
> for #1.
> 
> Once a thread's invalidation descriptors are complete, their status would
> be set to QI_FREE. The reclaim_free_desc() function would then only
> free descriptors marked as QI_FREE instead of those marked as
> QI_DONE. This change ensures that T2 (from the previous example) will
> correctly observe the completion of its invalidation_wait (marked as
> QI_DONE).
> 
> Signed-off-by: Sanjay K Kumar <sanjay.k.kumar@...el.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>
> 

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