dmaengine: ti: k3-udma: Fix NULL pointer dereference error
Subsystems (2)
v2 cover dmaengine: ti: k3-udma: Fix NULL pointer dereference error
NULL pointer de-reference error was observed when all the PCIe endpoint functions (22 function in J721E) request a DMA channel. The issue was specfically observed in BCDMA (Block copy DMA) but the issue is applicable in PKTDMA as well.
Changes from v1:
1) Split the patch for BCDMA and PKTDMA separately
2) Fixed the return value of udma_get_rflow() to 0.
3) Removed the fixes tag as the patches does not directly apply to the commits.
v1 => https://lore.kernel.org/r/[email protected]
Kishon Vijay Abraham I (2):
dmaengine: ti: k3-udma: Fix NULL pointer dereference error for BCDMA
dmaengine: ti: k3-udma: Fix NULL pointer dereference error for PKTDMA
drivers/dma/ti/k3-udma.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)
Signature · 1 line
2.17.1
PATCH RESEND v2 1/2 dmaengine: ti: k3-udma: Fix NULL pointer dereference error for BCDMA
bcdma_get_*() checks if bchan is already allocated by checking if it has a NON NULL value. For the error cases, bchan will have error value and bcdma_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.
Reset the value of bchan to NULL if the allocation actually fails.
--- drivers/dma/ti/k3-udma.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/dma/ti/k3-udma.c +6 -2
@@ -1348,6 +1348,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",
@@ -1365,8 +1366,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;
Signature · 1 line
2.17.1
PATCH RESEND v2 2/2 dmaengine: ti: k3-udma: Fix NULL pointer dereference error for PKTDMA
In reply to Kishon Vijay Abraham Iudma_get_*() checks if rchan/tchan/rflow is already allocated by checking if it has a NON NULL value. For the error cases, rchan/tchan/rflow will have error value 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 rchan/tchan/rflow.
Reset the value of rchan/tchan/rflow to NULL if the allocation actually fails.
--- drivers/dma/ti/k3-udma.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
drivers/dma/ti/k3-udma.c +20 -4
@@ -1380,6 +1380,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",
@@ -1394,8 +1395,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;
@@ -1425,6 +1429,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",
@@ -1439,8 +1444,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)
@@ -1494,6 +1504,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);
@@ -1507,8 +1518,13 @@ 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); +return 0 ; }static void bcdma_put_bchan (struct udma_chan *uc)
Signature · 1 line
2.17.1
Hi Kishon,
Quoted 4 lines · Kishon Vijay Abraham I
NULL pointer de-reference error was observed when all the PCIe endpoint functions (22 function in J721E) request a DMA channel. The issue was specfically observed in BCDMA (Block copy DMA) but the issue is applicable in PKTDMA as well.
Nice catch, interesting that it did not materialized before.
Can you re-word the patch subjects and commit messages accordingly?
This is not really BCDMA/PKTDMA issue but a missed uc->Xchan = NULL; which would cause the de-reference error.
Quoted 10 lines
Changes from v1:
1) Split the patch for BCDMA and PKTDMA separately
2) Fixed the return value ofudma_get_rflow()to 0.
3) Removed the fixes tag as the patches does not directly apply to the commits.v1 => https://lore.kernel.org/r/[email protected]
Kishon Vijay Abraham I (2):
dmaengine: ti: k3-udma: Fix NULL pointer dereference error for BCDMA
dmaengine: ti: k3-udma: Set bchan to NULL if a channel request fail
Quoted 1 line
dmaengine: ti: k3-udma: Fix NULL pointer dereference error for PKTDMA
dmaengine: ti: k3-udma: Set rchan/tchan to NULL if a channel request fail
This is also applicable for UDMA which only have rchan and tchan.
Quoted 4 lines
drivers/dma/ti/k3-udma.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)
Signature · 1 line
Péter