linux-kernel - [PATCH] iommu/amd: Fix amd_iommu_free

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: <20150203122551.GJ24151@twins.programming.kicks-ass.net>
Date:	Tue, 3 Feb 2015 13:25:51 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Benjamin LaHaise <bcrl@...ck.org>, linux-aio@...ck.org,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Joerg Roedel <joro@...tes.org>,
	Jesse Barnes <jbarnes@...tuousgeek.org>
Subject: [PATCH] iommu/amd: Fix amd_iommu_free_device()

On Tue, Feb 03, 2015 at 12:27:33PM +0100, Peter Zijlstra wrote:
> drivers/iommu/amd_iommu_v2.c-static void put_device_state_wait(struct device_state *dev_state)
> drivers/iommu/amd_iommu_v2.c-{
> drivers/iommu/amd_iommu_v2.c-   DEFINE_WAIT(wait);
> drivers/iommu/amd_iommu_v2.c-
> drivers/iommu/amd_iommu_v2.c-   prepare_to_wait(&dev_state->wq, &wait, TASK_UNINTERRUPTIBLE);
> drivers/iommu/amd_iommu_v2.c-   if (!atomic_dec_and_test(&dev_state->count))
> drivers/iommu/amd_iommu_v2.c:           schedule();
> drivers/iommu/amd_iommu_v2.c-   finish_wait(&dev_state->wq, &wait);
> drivers/iommu/amd_iommu_v2.c-
> drivers/iommu/amd_iommu_v2.c-   free_device_state(dev_state);
> drivers/iommu/amd_iommu_v2.c-}
> 
> No loop...

Jesse, any objections to this?

---
Subject: iommu/amd: Fix amd_iommu_free_device()

put_device_state_wait() doesn't loop on the condition and a spurious
wakeup will have it free the device state even though there might still
be references out to it.

Fix this by using 'normal' wait primitives.

Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
 drivers/iommu/amd_iommu_v2.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 90f70d0e1141..b6398d7285f7 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -151,18 +151,6 @@ static void put_device_state(struct device_state *dev_state)
 		wake_up(&dev_state->wq);
 }
 
-static void put_device_state_wait(struct device_state *dev_state)
-{
-	DEFINE_WAIT(wait);
-
-	prepare_to_wait(&dev_state->wq, &wait, TASK_UNINTERRUPTIBLE);
-	if (!atomic_dec_and_test(&dev_state->count))
-		schedule();
-	finish_wait(&dev_state->wq, &wait);
-
-	free_device_state(dev_state);
-}
-
 /* Must be called under dev_state->lock */
 static struct pasid_state **__get_pasid_state_ptr(struct device_state *dev_state,
 						  int pasid, bool alloc)
@@ -851,7 +839,13 @@ void amd_iommu_free_device(struct pci_dev *pdev)
 	/* Get rid of any remaining pasid states */
 	free_pasid_states(dev_state);
 
-	put_device_state_wait(dev_state);
+	put_device_state(dev_state);
+	/*
+	 * Wait until the last reference is dropped before freeing
+	 * the device state.
+	 */
+	wait_event(dev_state->wq, !atomic_read(&dev_state->count));
+	free_device_state(dev_state);
 }
 EXPORT_SYMBOL(amd_iommu_free_device);
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/