diff mbox

firmware: tegra: fix locking bugs in bpmp

Message ID 20170505053723.53qi4ob3df4ri2v4@mwanda
State Accepted
Headers show

Commit Message

Dan Carpenter May 5, 2017, 5:37 a.m. UTC 
There are a bunch of error paths were we don't unlock the bpmp->threaded
lock.  Also if __tegra_bpmp_channel_write() fails then we returned
success instead of an error code.

Fixes: 983de5f97169 ("firmware: tegra: Add BPMP support")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Thierry Reding June 13, 2017, 2:21 p.m. UTC | #1 
On Fri, May 05, 2017 at 08:37:23AM +0300, Dan Carpenter wrote:
> There are a bunch of error paths were we don't unlock the bpmp->threaded
> lock.  Also if __tegra_bpmp_channel_write() fails then we returned
> success instead of an error code.
> 
> Fixes: 983de5f97169 ("firmware: tegra: Add BPMP support")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied, though I did omit the err_ prefixes from the labels for
consistency.

Thierry
diff mbox

Patch

diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c
index 84e4c9a58a0c..dffdfb4e9da0 100644
--- a/drivers/firmware/tegra/bpmp.c
+++ b/drivers/firmware/tegra/bpmp.c
@@ -211,14 +211,17 @@  static ssize_t tegra_bpmp_channel_read(struct tegra_bpmp_channel *channel,
 	int index;
 
 	index = tegra_bpmp_channel_get_thread_index(channel);
-	if (index < 0)
-		return index;
+	if (index < 0) {
+		err = index;
+		goto unlock;
+	}
 
 	spin_lock_irqsave(&bpmp->lock, flags);
 	err = __tegra_bpmp_channel_read(channel, data, size);
 	clear_bit(index, bpmp->threaded.allocated);
 	spin_unlock_irqrestore(&bpmp->lock, flags);
 
+unlock:
 	up(&bpmp->threaded.lock);
 
 	return err;
@@ -256,35 +259,40 @@  tegra_bpmp_write_threaded(struct tegra_bpmp *bpmp, unsigned int mrq,
 
 	index = find_first_zero_bit(bpmp->threaded.allocated, count);
 	if (index == count) {
-		channel = ERR_PTR(-EBUSY);
-		goto unlock;
+		err = -EBUSY;
+		goto err_unlock;
 	}
 
 	channel = tegra_bpmp_channel_get_thread(bpmp, index);
 	if (!channel) {
-		channel = ERR_PTR(-EINVAL);
-		goto unlock;
+		err = -EINVAL;
+		goto err_unlock;
 	}
 
 	if (!tegra_bpmp_master_free(channel)) {
-		channel = ERR_PTR(-EBUSY);
-		goto unlock;
+		err = -EBUSY;
+		goto err_unlock;
 	}
 
 	set_bit(index, bpmp->threaded.allocated);
 
 	err = __tegra_bpmp_channel_write(channel, mrq, MSG_ACK | MSG_RING,
 					 data, size);
-	if (err < 0) {
-		clear_bit(index, bpmp->threaded.allocated);
-		goto unlock;
-	}
+	if (err < 0)
+		goto err_clear_allocated;
 
 	set_bit(index, bpmp->threaded.busy);
 
-unlock:
 	spin_unlock_irqrestore(&bpmp->lock, flags);
 	return channel;
+
+err_clear_allocated:
+	clear_bit(index, bpmp->threaded.allocated);
+err_unlock:
+	spin_unlock_irqrestore(&bpmp->lock, flags);
+	up(&bpmp->threaded.lock);
+
+	return ERR_PTR(err);
 }
 
 static ssize_t tegra_bpmp_channel_write(struct tegra_bpmp_channel *channel,