[PATCH] dmatest: Fix NULL pointer dereference on ioat

[PATCH] dmatest: Fix NULL pointer dereference on ioat

[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 <jon.mason@intel.com>
CC: Vinod Koul <vinod.koul@intel.com>
CC: Dan Williams <djbw@fb.com>
---
 drivers/dma/dmatest.c     |    4 ++--
 include/linux/dmaengine.h |    5 ++++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 24225f0..6ef9465 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -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);
 }
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index d3201e4..e0004fb 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -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,
-- 
1.7.9.5

Re: [PATCH] dmatest: Fix NULL pointer dereference on ioat

[viresh kumar]

On Sat, Nov 10, 2012 at 2:27 AM, Jon Mason <jon.mason@intel.com> wrote:
> 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 <jon.mason@intel.com>
> CC: Vinod Koul <vinod.koul@intel.com>
> CC: Dan Williams <djbw@fb.com>
> ---
>  drivers/dma/dmatest.c     |    4 ++--
>  include/linux/dmaengine.h |    5 ++++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
>

> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index d3201e4..e0004fb 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -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

Re: [PATCH] dmatest: Fix NULL pointer dereference on ioat

[Jon Mason]

On Sat, Nov 10, 2012 at 09:00:15AM +0530, viresh kumar wrote:
> On Sat, Nov 10, 2012 at 2:27 AM, Jon Mason <jon.mason@intel.com> wrote:
> > 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 <jon.mason@intel.com>
> > CC: Vinod Koul <vinod.koul@intel.com>
> > CC: Dan Williams <djbw@fb.com>
> > ---
> >  drivers/dma/dmatest.c     |    4 ++--
> >  include/linux/dmaengine.h |    5 ++++-
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> >
> 
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index d3201e4..e0004fb 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -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

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