| Message ID | 20191218190906.6641-1-john.stultz@linaro.org |
|---|---|
| State | Accepted |
| Commit | 2f42e05b942fe2fbfb9bbc6e34e1dd8c3ce4f3a4 |
| Headers | show |
| Series | k3dma: Avoid null pointer traversal | expand |
On 18-12-19, 19:09, John Stultz wrote: > In some cases we seem to submit two transactions in a row, which > causes us to lose track of the first. If we then cancel the > request, we may still get an interrupt, which traverses a null > ds_run value. > > So try to avoid starting a new transaction if the ds_run value > is set. > > While this patch avoids the null pointer crash, I've had some > reports of the k3dma driver still getting confused, which > suggests the ds_run/ds_done value handling still isn't quite > right. However, I've not run into an issue recently with it > so I think this patch is worth pushing upstream to avoid the > crash. Applied after adding dmaengine tag, thanks -- ~Vinod
diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c index adecea51814f..c5c1aa0dcaed 100644 --- a/drivers/dma/k3dma.c +++ b/drivers/dma/k3dma.c @@ -229,9 +229,11 @@ static irqreturn_t k3_dma_int_handler(int irq, void *dev_id) c = p->vchan; if (c && (tc1 & BIT(i))) { spin_lock_irqsave(&c->vc.lock, flags); - vchan_cookie_complete(&p->ds_run->vd); - p->ds_done = p->ds_run; - p->ds_run = NULL; + if (p->ds_run != NULL) { + vchan_cookie_complete(&p->ds_run->vd); + p->ds_done = p->ds_run; + p->ds_run = NULL; + } spin_unlock_irqrestore(&c->vc.lock, flags); } if (c && (tc2 & BIT(i))) { @@ -271,6 +273,10 @@ static int k3_dma_start_txd(struct k3_dma_chan *c) if (BIT(c->phy->idx) & k3_dma_get_chan_stat(d)) return -EAGAIN; + /* Avoid losing track of ds_run if a transaction is in flight */ + if (c->phy->ds_run) + return -EAGAIN; + if (vd) { struct k3_dma_desc_sw *ds = container_of(vd, struct k3_dma_desc_sw, vd);
In some cases we seem to submit two transactions in a row, which causes us to lose track of the first. If we then cancel the request, we may still get an interrupt, which traverses a null ds_run value. So try to avoid starting a new transaction if the ds_run value is set. While this patch avoids the null pointer crash, I've had some reports of the k3dma driver still getting confused, which suggests the ds_run/ds_done value handling still isn't quite right. However, I've not run into an issue recently with it so I think this patch is worth pushing upstream to avoid the crash. Cc: Vinod Koul <vkoul@kernel.org> Cc: Dan Williams <dan.j.williams@intel.com> Cc: ryan@edited.us Cc: aserbinski@gmail.com Cc: dmaengine@vger.kernel.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- drivers/dma/k3dma.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) -- 2.17.1