Skip to contentSkip to conversation

spi: add null check before pointer dereference

PATCH In review Gustavo A. R. Silva 4 participants 5 messages May 23, 2017 – May 30, 2017

Subsystems (3)
SPI Arm Samsung Exynos Samsung SPI drivers
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 spi: add null check before pointer dereference

Add null check before dereferencing pointer desc

Addresses-Coverity-ID: 1397997

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/spi/spi-s3c64xx.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/spi/spi-s3c64xx.c +6 -0
index b392cca..9f1013e 100644
@@ -306,6 +306,12 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
 	desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
 				       dma->direction, DMA_PREP_INTERRUPT);
 
+	if (!desc) {
+		dev_err(&sdd->master->dev,
+			"%s:dmaengine_prep_slave_sg Failed\n", __func__);
+		return;
+	}
+
 	desc->callback = s3c64xx_spi_dmacb;
 	desc->callback_param = dma;
Signature · 1 line
2.5.0
maintainer
#1 lore raw
Quoted 13 lines · Gustavo A. R. Silva

Add null check before dereferencing pointer desc

Addresses-Coverity-ID: 1397997

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/spi/spi-s3c64xx.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/spi/spi-s3c64xx.c +0 -0
index b392cca..9f1013e 100644

drivers/spi/spi-s3c64xx.c static void prepare_dma(struct s3c64xx_spi_dma_data *dma, · line 313

Quoted 8 lines
drivers/spi/spi-s3c64xx.c +5 -0
@@ -306,6 +306,12 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
        desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
                                       dma->direction, DMA_PREP_INTERRUPT);
 
+       if (!desc) {
+               dev_err(&sdd->master->dev,
+                       "%s:dmaengine_prep_slave_sg Failed\n", __func__);
+               return;
+       }

Although the check itself looks needed, but I think you did not handle the error at all. You just bail out before submitting dma descriptor but except that, everything else goes like there was no error. It might work, might not... did you test this error path? How does it behave?

Best regards,
Krzysztof

#2 lore raw
Quoted 18 lines · Krzysztof Kozlowski
Quoted 16 lines · Gustavo A. R. Silva

Add null check before dereferencing pointer desc

Addresses-Coverity-ID: 1397997

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/spi/spi-s3c64xx.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/spi/spi-s3c64xx.c +0 -0
index b392cca..9f1013e 100644
@@ -306,6 +306,12 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
        desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
                                       dma->direction, DMA_PREP_INTERRUPT);
Quoted 11 lines

drivers/spi/spi-s3c64xx.c static void prepare_dma(struct s3c64xx_spi_dma_data *dma, · line 313

Quoted 5 lines
drivers/spi/spi-s3c64xx.c +5 -0
+       if (!desc) {
+               dev_err(&sdd->master->dev,
+                       "%s:dmaengine_prep_slave_sg Failed\n", __func__);
+               return;
+       }

Although the check itself looks needed, but I think you did not handle the error at all. You just bail out before submitting dma descriptor but except that, everything else goes like there was no error. It might work, might not... did you test this error path? How does it behave?

I.e. does it fall back to PIO?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

#3 lore raw
In reply to Gustavo A. R. Silva

Hi Gustavo,

drivers/spi/spi-s3c64xx.c static void prepare_dma(struct s3c64xx_spi_dma_data *dma, · line 314

Quoted 9 lines
drivers/spi/spi-s3c64xx.c +6 -0
 	desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
 				       dma->direction, DMA_PREP_INTERRUPT);
 
+	if (!desc) {
+		dev_err(&sdd->master->dev,
+			"%s:dmaengine_prep_slave_sg Failed\n", __func__);
+		return;
+	}
+

I'm sorry, I would nack this patch for now. There was a smilar I sent before, but, as Krzysztof said, this needs more testing and a proper solution.

That's anyway in my todo list.

Thanks,
Andi

author
#4 lore raw

Hi Andi,

Quoting Andi Shyti <[email protected]>:

Quoted 5 lines

Hi Gustavo,

Quoted 3 lines
drivers/spi/spi-s3c64xx.c +0 -0
 	desc = dmaengine_prep_slave_sg(dma->ch, sgt->sgl, sgt->nents,
 				       dma->direction, DMA_PREP_INTERRUPT);
Quoted 11 lines

drivers/spi/spi-s3c64xx.c static void prepare_dma(struct s3c64xx_spi_dma_data *dma, · line 314

Quoted 6 lines
drivers/spi/spi-s3c64xx.c +6 -0
+	if (!desc) {
+		dev_err(&sdd->master->dev,
+			"%s:dmaengine_prep_slave_sg Failed\n", __func__);
+		return;
+	}
+

I'm sorry, I would nack this patch for now. There was a smilar I sent before, but, as Krzysztof said, this needs more testing and a proper solution.

Yeah, I get it.

Quoted 2 lines

That's anyway in my todo list.

That's great.

Quoted 2 lines

Thanks,
Andi

Thanks!
--
Gustavo A. R. Silva