Skip to contentSkip to conversation

dmaengine: ti: k3-udma: Fix NULL pointer dereference error

PATCH Superseded Kishon Vijay Abraham I 2 participants 3 messages Feb 9, 2021 – Feb 10, 2021

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 dmaengine: ti: k3-udma: Fix NULL pointer dereference error

bcdma_get_*() and udma_get_*() checks if bchan/rchan/tchan/rflow is already allocated by checking if it has a NON NULL value. For the error cases, bchan/rchan/tchan/rflow will have error value and bcdma_get_*() and udma_get_*() considers this as already allocated (PASS) since the error values are NON NULL. This results in NULL pointer dereference error while de-referencing bchan/rchan/tchan/rflow.

Reset the value of bchan/rchan/tchan/rflow to NULL if the allocation actually fails.

Fixes 017794739702 ("dmaengine: ti: k3-udma: Initial support for K3 BCDMA")
Fixes 25dcb5dd7b7c ("dmaengine: ti: New driver for K3 UDMA")
---
 drivers/dma/ti/k3-udma.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)
drivers/dma/ti/k3-udma.c +25 -5
index 298460438bb4..aa4ef583ff83 100644
@@ -1330,6 +1330,7 @@ static int bcdma_get_bchan(struct udma_chan *uc)
 {
 	struct udma_dev *ud = uc->ud;
 	enum udma_tp_level tpl;
+	int ret;
 
 	if (uc->bchan) {
 		dev_dbg(ud->dev, "chan%d: already have bchan%d allocated\n",
@@ -1347,8 +1348,11 @@ static int bcdma_get_bchan(struct udma_chan *uc)
 		tpl = ud->bchan_tpl.levels - 1;
 
 	uc->bchan = __udma_reserve_bchan(ud, tpl, -1);
-	if (IS_ERR(uc->bchan))
-		return PTR_ERR(uc->bchan);
+	if (IS_ERR(uc->bchan)) {
+		ret = PTR_ERR(uc->bchan);
+		uc->bchan = NULL;
+		return ret;
+	}
 
 	uc->tchan = uc->bchan;
 
@@ -1358,6 +1362,7 @@ static int bcdma_get_bchan(struct udma_chan *uc)
 static int udma_get_tchan(struct udma_chan *uc)
 {
 	struct udma_dev *ud = uc->ud;
+	int ret;
 
 	if (uc->tchan) {
 		dev_dbg(ud->dev, "chan%d: already have tchan%d allocated\n",
@@ -1372,8 +1377,11 @@ static int udma_get_tchan(struct udma_chan *uc)
 	 */
 	uc->tchan = __udma_reserve_tchan(ud, uc->config.channel_tpl,
 					 uc->config.mapped_channel_id);
-	if (IS_ERR(uc->tchan))
-		return PTR_ERR(uc->tchan);
+	if (IS_ERR(uc->tchan)) {
+		ret = PTR_ERR(uc->tchan);
+		uc->tchan = NULL;
+		return ret;
+	}
 
 	if (ud->tflow_cnt) {
 		int tflow_id;
@@ -1403,6 +1411,7 @@ static int udma_get_tchan(struct udma_chan *uc)
 static int udma_get_rchan(struct udma_chan *uc)
 {
 	struct udma_dev *ud = uc->ud;
+	int ret;
 
 	if (uc->rchan) {
 		dev_dbg(ud->dev, "chan%d: already have rchan%d allocated\n",
@@ -1417,8 +1426,13 @@ static int udma_get_rchan(struct udma_chan *uc)
 	 */
 	uc->rchan = __udma_reserve_rchan(ud, uc->config.channel_tpl,
 					 uc->config.mapped_channel_id);
+	if (IS_ERR(uc->rchan)) {
+		ret = PTR_ERR(uc->rchan);
+		uc->rchan = NULL;
+		return ret;
+	}
 
-	return PTR_ERR_OR_ZERO(uc->rchan);
+	return 0;
 }
 
 static int udma_get_chan_pair(struct udma_chan *uc)
@@ -1472,6 +1486,7 @@ static int udma_get_chan_pair(struct udma_chan *uc)
 static int udma_get_rflow(struct udma_chan *uc, int flow_id)
 {
 	struct udma_dev *ud = uc->ud;
+	int ret;
 
 	if (!uc->rchan) {
 		dev_err(ud->dev, "chan%d: does not have rchan??\n", uc->id);
@@ -1485,6 +1500,11 @@ static int udma_get_rflow(struct udma_chan *uc, int flow_id)
 	}
 
 	uc->rflow = __udma_get_rflow(ud, flow_id);
+	if (IS_ERR(uc->rflow)) {
+		ret = PTR_ERR(uc->rflow);
+		uc->rflow = NULL;
+		return ret;
+	}
 
 	return PTR_ERR_OR_ZERO(uc->rflow);
 }
Signature · 1 line
2.17.1
#1 lore raw

Hi Kishon,

Quoted 8 lines · Kishon Vijay Abraham I

bcdma_get_*() and udma_get_*() checks if bchan/rchan/tchan/rflow is already allocated by checking if it has a NON NULL value. For the error cases, bchan/rchan/tchan/rflow will have error value and bcdma_get_*() and udma_get_*() considers this as already allocated (PASS) since the error values are NON NULL. This results in NULL pointer dereference error while de-referencing bchan/rchan/tchan/rflow.

Quoted 6 lines

Reset the value of bchan/rchan/tchan/rflow to NULL if the allocation actually fails.

Fixes 017794739702 ("dmaengine: ti: k3-udma: Initial support for K3 BCDMA")
Fixes 25dcb5dd7b7c ("dmaengine: ti: New driver for K3 UDMA")

Is this the same patch as the other with the similar subject?

Signature · 1 line
Péter
Quoted 96 lines
---
 drivers/dma/ti/k3-udma.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)
drivers/dma/ti/k3-udma.c +25 -5
index 298460438bb4..aa4ef583ff83 100644
@@ -1330,6 +1330,7 @@ static int bcdma_get_bchan(struct udma_chan *uc)
 {
 	struct udma_dev *ud = uc->ud;
 	enum udma_tp_level tpl;
+	int ret;
 
 	if (uc->bchan) {
 		dev_dbg(ud->dev, "chan%d: already have bchan%d allocated\n",
@@ -1347,8 +1348,11 @@ static int bcdma_get_bchan(struct udma_chan *uc)
 		tpl = ud->bchan_tpl.levels - 1;
 
 	uc->bchan = __udma_reserve_bchan(ud, tpl, -1);
-	if (IS_ERR(uc->bchan))
-		return PTR_ERR(uc->bchan);
+	if (IS_ERR(uc->bchan)) {
+		ret = PTR_ERR(uc->bchan);
+		uc->bchan = NULL;
+		return ret;
+	}
 
 	uc->tchan = uc->bchan;
 
@@ -1358,6 +1362,7 @@ static int bcdma_get_bchan(struct udma_chan *uc)
 static int udma_get_tchan(struct udma_chan *uc)
 {
 	struct udma_dev *ud = uc->ud;
+	int ret;
 
 	if (uc->tchan) {
 		dev_dbg(ud->dev, "chan%d: already have tchan%d allocated\n",
@@ -1372,8 +1377,11 @@ static int udma_get_tchan(struct udma_chan *uc)
 	 */
 	uc->tchan = __udma_reserve_tchan(ud, uc->config.channel_tpl,
 					 uc->config.mapped_channel_id);
-	if (IS_ERR(uc->tchan))
-		return PTR_ERR(uc->tchan);
+	if (IS_ERR(uc->tchan)) {
+		ret = PTR_ERR(uc->tchan);
+		uc->tchan = NULL;
+		return ret;
+	}
 
 	if (ud->tflow_cnt) {
 		int tflow_id;
@@ -1403,6 +1411,7 @@ static int udma_get_tchan(struct udma_chan *uc)
 static int udma_get_rchan(struct udma_chan *uc)
 {
 	struct udma_dev *ud = uc->ud;
+	int ret;
 
 	if (uc->rchan) {
 		dev_dbg(ud->dev, "chan%d: already have rchan%d allocated\n",
@@ -1417,8 +1426,13 @@ static int udma_get_rchan(struct udma_chan *uc)
 	 */
 	uc->rchan = __udma_reserve_rchan(ud, uc->config.channel_tpl,
 					 uc->config.mapped_channel_id);
+	if (IS_ERR(uc->rchan)) {
+		ret = PTR_ERR(uc->rchan);
+		uc->rchan = NULL;
+		return ret;
+	}
 
-	return PTR_ERR_OR_ZERO(uc->rchan);
+	return 0;
 }
 
 static int udma_get_chan_pair(struct udma_chan *uc)
@@ -1472,6 +1486,7 @@ static int udma_get_chan_pair(struct udma_chan *uc)
 static int udma_get_rflow(struct udma_chan *uc, int flow_id)
 {
 	struct udma_dev *ud = uc->ud;
+	int ret;
 
 	if (!uc->rchan) {
 		dev_err(ud->dev, "chan%d: does not have rchan??\n", uc->id);
@@ -1485,6 +1500,11 @@ static int udma_get_rflow(struct udma_chan *uc, int flow_id)
 	}
 
 	uc->rflow = __udma_get_rflow(ud, flow_id);
+	if (IS_ERR(uc->rflow)) {
+		ret = PTR_ERR(uc->rflow);
+		uc->rflow = NULL;
+		return ret;
+	}
 
 	return PTR_ERR_OR_ZERO(uc->rflow);
 }
#2 lore raw

Hi Peter,

Quoted 11 lines · Péter Ujfalusi

Hi Kishon,

Quoted 8 lines · Kishon Vijay Abraham I

bcdma_get_*() and udma_get_*() checks if bchan/rchan/tchan/rflow is already allocated by checking if it has a NON NULL value. For the error cases, bchan/rchan/tchan/rflow will have error value and bcdma_get_*() and udma_get_*() considers this as already allocated (PASS) since the error values are NON NULL. This results in NULL pointer dereference error while de-referencing bchan/rchan/tchan/rflow.

Quoted 8 lines
Quoted 6 lines

Reset the value of bchan/rchan/tchan/rflow to NULL if the allocation actually fails.

Fixes 017794739702 ("dmaengine: ti: k3-udma: Initial support for K3 BCDMA")
Fixes 25dcb5dd7b7c ("dmaengine: ti: New driver for K3 UDMA")

Is this the same patch as the other with the similar subject?

Right, sorry for the duplicate patch (thought it didn't go out initially).

Regards
Kishon