| Message ID | 20230127223623.never.507-kees@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | efa8c2ee66ad95c00ea591f95c163db68936da6b |
| Headers | show |
| Series | dmaengine: dw-axi-dmac: Do not dereference NULL structure | expand |
On Fri, Jan 27, 2023 at 02:36:27PM -0800, Kees Cook wrote: > If "vdesc" is NULL, it cannot be used with vd_to_axi_desc(). Leave > "bytes" unchanged at 0. Seen under GCC 13 with -Warray-bounds: > > ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c: In function 'dma_chan_tx_status': > ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:329:46: warning: array subscript 0 is outside array bounds of 'struct > virt_dma_desc[46116860184273879]' [-Warray-bounds=] > 329 | bytes = vd_to_axi_desc(vdesc)->length; > | ^~ > > Fixes: 8e55444da65c ("dmaengine: dw-axi-dmac: Support burst residue granularity") > Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: dmaengine@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 08af483331fd..4169e1d7d5ca 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -325,8 +325,6 @@ dma_chan_tx_status(struct dma_chan *dchan, dma_cookie_t cookie, > len = vd_to_axi_desc(vdesc)->hw_desc[0].len; > completed_length = completed_blocks * len; > bytes = length - completed_length; > - } else { > - bytes = vd_to_axi_desc(vdesc)->length; > } > > spin_unlock_irqrestore(&chan->vc.lock, flags); If you want to fix it properly, the code should be modified like status = dma_cookie_status(dchan, cookie, txstate); if (status == DMA_COMPLETE || !txstate) { if (txstate) goto out; return status; } ... out: dma_set_residue(txstate, bytes); return status; to be in accordance with the Documentation. **OR** the Documentation should be fixed to tell that if status is DMA_COMPLETE, residue is undefined and assumed to be 0. Vinod?
On Mon, Jan 30, 2023 at 12:46:26PM +0200, Andy Shevchenko wrote: > On Fri, Jan 27, 2023 at 02:36:27PM -0800, Kees Cook wrote: ... > If you want to fix it properly, the code should be modified like > > status = dma_cookie_status(dchan, cookie, txstate); > if (status == DMA_COMPLETE || !txstate) { > if (txstate) > goto out; > return status; > } > > ... > > out: > dma_set_residue(txstate, bytes); > > return status; > > to be in accordance with the Documentation. > > **OR** > > the Documentation should be fixed to tell that if status is DMA_COMPLETE, > residue is undefined and assumed to be 0. > > Vinod? Disregard my message. The dma_cookie_status() makes sure it's 0. Sorry for the noise.
On Fri, Jan 27, 2023 at 02:36:27PM -0800, Kees Cook wrote: > If "vdesc" is NULL, it cannot be used with vd_to_axi_desc(). Leave > "bytes" unchanged at 0. Seen under GCC 13 with -Warray-bounds: > > ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c: In function 'dma_chan_tx_status': > ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:329:46: warning: array subscript 0 is outside array bounds of 'struct > virt_dma_desc[46116860184273879]' [-Warray-bounds=] > 329 | bytes = vd_to_axi_desc(vdesc)->length; > | ^~ Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Fixes: 8e55444da65c ("dmaengine: dw-axi-dmac: Support burst residue granularity") > Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > Cc: Vinod Koul <vkoul@kernel.org> > Cc: dmaengine@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > index 08af483331fd..4169e1d7d5ca 100644 > --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c > @@ -325,8 +325,6 @@ dma_chan_tx_status(struct dma_chan *dchan, dma_cookie_t cookie, > len = vd_to_axi_desc(vdesc)->hw_desc[0].len; > completed_length = completed_blocks * len; > bytes = length - completed_length; > - } else { > - bytes = vd_to_axi_desc(vdesc)->length; > } > > spin_unlock_irqrestore(&chan->vc.lock, flags); > -- > 2.34.1 >
On 27-01-23, 14:36, Kees Cook wrote: > If "vdesc" is NULL, it cannot be used with vd_to_axi_desc(). Leave > "bytes" unchanged at 0. Seen under GCC 13 with -Warray-bounds: > > ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c: In function 'dma_chan_tx_status': > ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:329:46: warning: array subscript 0 is outside array bounds of 'struct > virt_dma_desc[46116860184273879]' [-Warray-bounds=] > 329 | bytes = vd_to_axi_desc(vdesc)->length; > | ^~ > Applied, thanks
diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c index 08af483331fd..4169e1d7d5ca 100644 --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c @@ -325,8 +325,6 @@ dma_chan_tx_status(struct dma_chan *dchan, dma_cookie_t cookie, len = vd_to_axi_desc(vdesc)->hw_desc[0].len; completed_length = completed_blocks * len; bytes = length - completed_length; - } else { - bytes = vd_to_axi_desc(vdesc)->length; } spin_unlock_irqrestore(&chan->vc.lock, flags);
If "vdesc" is NULL, it cannot be used with vd_to_axi_desc(). Leave "bytes" unchanged at 0. Seen under GCC 13 with -Warray-bounds: ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c: In function 'dma_chan_tx_status': ../drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:329:46: warning: array subscript 0 is outside array bounds of 'struct virt_dma_desc[46116860184273879]' [-Warray-bounds=] 329 | bytes = vd_to_axi_desc(vdesc)->length; | ^~ Fixes: 8e55444da65c ("dmaengine: dw-axi-dmac: Support burst residue granularity") Cc: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> Cc: Vinod Koul <vkoul@kernel.org> Cc: dmaengine@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c | 2 -- 1 file changed, 2 deletions(-)