| Message ID | 1460476049-11013-1-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 12/04/16 16:47, Vinod Koul wrote: > dmaengine has various device callbacks and exposes helper > functions to invoke these. These helpers should check if channel, > device and callback is valid or not before invoking them. > > Reported-by: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > --- > include/linux/dmaengine.h | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index 017433712833..30de0197263a 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -804,6 +804,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single( > sg_dma_address(&sg) = buf; > sg_dma_len(&sg) = len; > > + if (!chan || !chan->device || !chan->device->device_prep_slave_sg) > + return NULL; > + > return chan->device->device_prep_slave_sg(chan, &sg, 1, > dir, flags, NULL); > } > @@ -812,6 +815,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg( > struct dma_chan *chan, struct scatterlist *sgl, unsigned int sg_len, > enum dma_transfer_direction dir, unsigned long flags) > { > + if (!chan || !chan->device || !chan->device->device_prep_slave_sg) > + return NULL; > + > return chan->device->device_prep_slave_sg(chan, sgl, sg_len, > dir, flags, NULL); > } > @@ -823,6 +829,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_rio_sg( > enum dma_transfer_direction dir, unsigned long flags, > struct rio_dma_ext *rio_ext) > { > + if (!chan || !chan->device || !chan->device->device_prep_slave_sg) > + return NULL; > + > return chan->device->device_prep_slave_sg(chan, sgl, sg_len, > dir, flags, rio_ext); > } > @@ -833,6 +842,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_cyclic( > size_t period_len, enum dma_transfer_direction dir, > unsigned long flags) > { > + if (!chan || !chan->device || !chan->device->device_prep_dma_cyclic) > + return NULL; > + > return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len, > period_len, dir, flags); > } > @@ -841,6 +853,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma( > struct dma_chan *chan, struct dma_interleaved_template *xt, > unsigned long flags) > { > + if (!chan || !chan->device || !chan->device->device_prep_interleaved_dma) > + return NULL; > + > return chan->device->device_prep_interleaved_dma(chan, xt, flags); > } > > @@ -848,7 +863,7 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memset( > struct dma_chan *chan, dma_addr_t dest, int value, size_t len, > unsigned long flags) > { > - if (!chan || !chan->device) > + if (!chan || !chan->device || !chan->device->device_prep_dma_memset) > return NULL; > > return chan->device->device_prep_dma_memset(chan, dest, value, > @@ -861,6 +876,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_sg( > struct scatterlist *src_sg, unsigned int src_nents, > unsigned long flags) > { > + if (!chan || !chan->device || !chan->device->device_prep_dma_sg) > + return NULL; > + > return chan->device->device_prep_dma_sg(chan, dst_sg, dst_nents, > src_sg, src_nents, flags); > } My only thought here is if the pointer for chan is valid, should this imply that the pointer for chan->device should also be valid? In other words, do we need to check for both? I see that today in dma_async_device_register() we do not check that chan->device is populated for each channel, but I am wondering if we should and if it is not return an error? Cheers Jon ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. ----------------------------------------------------------------------------------- -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 12, 2016 at 06:39:37PM +0100, Jon Hunter wrote: > > My only thought here is if the pointer for chan is valid, should this > imply that the pointer for chan->device should also be valid? In other > words, do we need to check for both? I suppose this would be a fair assumption that chan->device would be valid as long as you get a valid channel. > I see that today in dma_async_device_register() we do not check that > chan->device is populated for each channel, but I am wondering if we > should and if it is not return an error? Without this nothing will work, so you would essentially get a broken one. So don't think it is worth checking > This email message is for the sole use of the intended recipient(s) and may contain > confidential information. Any unauthorized review, use, disclosure or distribution > is prohibited. If you are not the intended recipient, please contact the sender by > reply email and destroy all copies of the original message. ummmmm? Thanks
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index 017433712833..30de0197263a 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -804,6 +804,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single( sg_dma_address(&sg) = buf; sg_dma_len(&sg) = len; + if (!chan || !chan->device || !chan->device->device_prep_slave_sg) + return NULL; + return chan->device->device_prep_slave_sg(chan, &sg, 1, dir, flags, NULL); } @@ -812,6 +815,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_sg( struct dma_chan *chan, struct scatterlist *sgl, unsigned int sg_len, enum dma_transfer_direction dir, unsigned long flags) { + if (!chan || !chan->device || !chan->device->device_prep_slave_sg) + return NULL; + return chan->device->device_prep_slave_sg(chan, sgl, sg_len, dir, flags, NULL); } @@ -823,6 +829,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_rio_sg( enum dma_transfer_direction dir, unsigned long flags, struct rio_dma_ext *rio_ext) { + if (!chan || !chan->device || !chan->device->device_prep_slave_sg) + return NULL; + return chan->device->device_prep_slave_sg(chan, sgl, sg_len, dir, flags, rio_ext); } @@ -833,6 +842,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_cyclic( size_t period_len, enum dma_transfer_direction dir, unsigned long flags) { + if (!chan || !chan->device || !chan->device->device_prep_dma_cyclic) + return NULL; + return chan->device->device_prep_dma_cyclic(chan, buf_addr, buf_len, period_len, dir, flags); } @@ -841,6 +853,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_interleaved_dma( struct dma_chan *chan, struct dma_interleaved_template *xt, unsigned long flags) { + if (!chan || !chan->device || !chan->device->device_prep_interleaved_dma) + return NULL; + return chan->device->device_prep_interleaved_dma(chan, xt, flags); } @@ -848,7 +863,7 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_memset( struct dma_chan *chan, dma_addr_t dest, int value, size_t len, unsigned long flags) { - if (!chan || !chan->device) + if (!chan || !chan->device || !chan->device->device_prep_dma_memset) return NULL; return chan->device->device_prep_dma_memset(chan, dest, value, @@ -861,6 +876,9 @@ static inline struct dma_async_tx_descriptor *dmaengine_prep_dma_sg( struct scatterlist *src_sg, unsigned int src_nents, unsigned long flags) { + if (!chan || !chan->device || !chan->device->device_prep_dma_sg) + return NULL; + return chan->device->device_prep_dma_sg(chan, dst_sg, dst_nents, src_sg, src_nents, flags); }
dmaengine has various device callbacks and exposes helper functions to invoke these. These helpers should check if channel, device and callback is valid or not before invoking them. Reported-by: Jon Hunter <jonathanh@nvidia.com> Signed-off-by: Vinod Koul <vinod.koul@intel.com> --- include/linux/dmaengine.h | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)