[PATCH linux dev-4.10 1/3] drivers: fsi: sbefifo: Fix queued xfrs race condition

[Eddie James]

From: "Edward A. James" <eajames at us.ibm.com>

There was a race condition where: sbefifo remove() is called, a transfer
is queued, a client is waiting, and a client is released at the same
time. In this situation, remove() wakes up the client waiter, which
destroys the client's list of transfers. This must be done since in an
error case, we don't know if the transfers are deleted or not. Then,
the client is released. Since it has no transfers, it fails to cancel
the queued transfer. This transfer then wakes up on the timer, where it
attempts to access it's client buffers, which have been freed.

Signed-off-by: Edward A. James <eajames at us.ibm.com>
---
 drivers/fsi/fsi-sbefifo.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 5d73437..c914eaf 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -312,22 +312,27 @@ static struct sbefifo_client *sbefifo_new_client(struct sbefifo *sbefifo)
 
 static void sbefifo_client_release(struct kref *kref)
 {
+	struct sbefifo *sbefifo;
 	struct sbefifo_client *client;
 	struct sbefifo_xfr *xfr;
 
 	client = container_of(kref, struct sbefifo_client, kref);
-	list_for_each_entry(xfr, &client->xfrs, client) {
-		/*
-		 * The client left with pending or running xfrs.
-		 * Cancel them.
-		 */
-		set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
-		sbefifo_get(client->dev);
-		if (mod_timer(&client->dev->poll_timer, jiffies))
-			sbefifo_put(client->dev);
+	sbefifo = client->dev;
+
+	if (!READ_ONCE(sbefifo->rc)) {
+		list_for_each_entry(xfr, &client->xfrs, client) {
+			/*
+			 * The client left with pending or running xfrs.
+			 * Cancel them.
+			 */
+			set_bit(SBEFIFO_XFR_CANCEL, &xfr->flags);
+			sbefifo_get(sbefifo);
+			if (mod_timer(&client->dev->poll_timer, jiffies))
+				sbefifo_put(sbefifo);
+		}
 	}
 
-	sbefifo_put(client->dev);
+	sbefifo_put(sbefifo);
 	kfree(client);
 }
 
@@ -901,7 +906,16 @@ static int sbefifo_remove(struct device *dev)
 	struct sbefifo *sbefifo = dev_get_drvdata(dev);
 	struct sbefifo_xfr *xfr;
 
+	spin_lock(&sbefifo->lock);
+
 	WRITE_ONCE(sbefifo->rc, -ENODEV);
+	list_for_each_entry(xfr, &sbefifo->xfrs, xfrs)
+		kfree(xfr);
+
+	INIT_LIST_HEAD(&sbefifo->xfrs);
+
+	spin_unlock(&sbefifo->lock);
+
 	wake_up_all(&sbefifo->wait);
 
 	misc_deregister(&sbefifo->mdev);
@@ -912,11 +926,6 @@ static int sbefifo_remove(struct device *dev)
 	if (del_timer_sync(&sbefifo->poll_timer))
 		sbefifo_put(sbefifo);
 
-	spin_lock(&sbefifo->lock);
-	list_for_each_entry(xfr, &sbefifo->xfrs, xfrs)
-		kfree(xfr);
-	spin_unlock(&sbefifo->lock);
-
 	sbefifo_put(sbefifo);
 
 	return 0;
-- 
1.8.3.1