Skip to contentSkip to conversation

dmatest: Fix NULL pointer dereference on ioat

PATCH Superseded Jon Mason 2 participants 3 messages Nov 9, 2012 – Nov 11, 2012 dmaengine

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jon Mason

author
#0 lore raw

PATCH 1/1 dmatest: Fix NULL pointer dereference on ioat

device_control is an optional and not implemented in all DMA drivers. Any calls to these will result in a NULL pointer dereference. dmatest makes two of these calls when completing the kernel thread and removing the module. These are corrected by calling the dmaengine_device_control wrapper and checking for a non-existant device_control function pointer there.

Signed-off-by Jon Mason <[email protected]>
CC Vinod Koul <[email protected]>
CC Dan Williams <[email protected]>
---
 drivers/dma/dmatest.c     |    4 ++--
 include/linux/dmaengine.h |    5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)
drivers/dma/dmatest.c +2 -2
index 24225f0..6ef9465 100644
@@ -499,7 +499,7 @@ err_srcs:
 			thread_name, total_tests, failed_tests, ret);
 
 	/* terminate all transfers on specified channels */
-	chan->device->device_control(chan, DMA_TERMINATE_ALL, 0);
+	dmaengine_terminate_all(chan);
 	if (iterations > 0)
 		while (!kthread_should_stop()) {
 			DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wait_dmatest_exit);

@@ -524,7 +524,7 @@ static void dmatest_cleanup_channel(struct dmatest_chan *dtc)
 	}
 
 	/* terminate all transfers on specified channels */
-	dtc->chan->device->device_control(dtc->chan, DMA_TERMINATE_ALL, 0);
+	dmaengine_terminate_all(dtc->chan);
 
 	kfree(dtc);
 }
include/linux/dmaengine.h +4 -1
index d3201e4..e0004fb 100644
@@ -608,7 +608,10 @@ static inline int dmaengine_device_control(struct dma_chan *chan,
 					   enum dma_ctrl_cmd cmd,
 					   unsigned long arg)
 {
-	return chan->device->device_control(chan, cmd, arg);
+	if (chan->device->device_control)
+		return chan->device->device_control(chan, cmd, arg);
+	else
+		return -EINVAL;
 }
 
 static inline int dmaengine_slave_config(struct dma_chan *chan,
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 21 lines · Jon Mason

device_control is an optional and not implemented in all DMA drivers. Any calls to these will result in a NULL pointer dereference. dmatest makes two of these calls when completing the kernel thread and removing the module. These are corrected by calling the dmaengine_device_control wrapper and checking for a non-existant device_control function pointer there.

Signed-off-by Jon Mason <[email protected]>
CC Vinod Koul <[email protected]>
CC Dan Williams <[email protected]>
---
 drivers/dma/dmatest.c     |    4 ++--
 include/linux/dmaengine.h |    5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)
include/linux/dmaengine.h +0 -0
index d3201e4..e0004fb 100644

include/linux/dmaengine.h static inline int dmaengine_device_control(struct dma_chan *chan, · line 614

Quoted 8 lines
include/linux/dmaengine.h +4 -1
@@ -608,7 +608,10 @@ static inline int dmaengine_device_control(struct dma_chan *chan,
                                           enum dma_ctrl_cmd cmd,
                                           unsigned long arg)
 {
-       return chan->device->device_control(chan, cmd, arg);
+       if (chan->device->device_control)
+               return chan->device->device_control(chan, cmd, arg);
+       else
+               return -EINVAL;

-ENOTSUPP or -ENOSYS ??

@Dan: I believe i don't have to send another version now. Correct??

--
viresh

author
#2 lore raw
Quoted 17 lines · viresh kumar
Quoted 15 lines · Jon Mason

device_control is an optional and not implemented in all DMA drivers. Any calls to these will result in a NULL pointer dereference. dmatest makes two of these calls when completing the kernel thread and removing the module. These are corrected by calling the dmaengine_device_control wrapper and checking for a non-existant device_control function pointer there.

Signed-off-by Jon Mason <[email protected]>
CC Vinod Koul <[email protected]>
CC Dan Williams <[email protected]>
---
 drivers/dma/dmatest.c     |    4 ++--
 include/linux/dmaengine.h |    5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)
Quoted 15 lines

include/linux/dmaengine.h static inline int dmaengine_device_control(struct dma_chan *chan, · line 614

Quoted 13 lines
include/linux/dmaengine.h +4 -1
index d3201e4..e0004fb 100644
@@ -608,7 +608,10 @@ static inline int dmaengine_device_control(struct dma_chan *chan,
                                           enum dma_ctrl_cmd cmd,
                                           unsigned long arg)
 {
-       return chan->device->device_control(chan, cmd, arg);
+       if (chan->device->device_control)
+               return chan->device->device_control(chan, cmd, arg);
+       else
+               return -EINVAL;

-ENOTSUPP or -ENOSYS ??

ENOSYS makes more sense to me. I'll send out another version shortly.

Dan, do you want me to send the patch to Greg KH for inclusion in the stable branches back to v3.2?

Thanks,
Jon

Quoted 5 lines

@Dan: I believe i don't have to send another version now. Correct??

--
viresh