[PATCH] spi: add null check before pointer dereference

[PATCH] spi: add null check before pointer dereference

[Gustavo A. R. Silva]

Add null check before dereferencing pointer desc

Addresses-Coverity-ID: 1397997
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/spi/spi-s3c64xx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index b392cca..9f1013e 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -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;
 
-- 
2.5.0

[PATCH] spi: add null check before pointer dereference

[Gustavo A. R. Silva]

Add null check before dereferencing pointer desc

Addresses-Coverity-ID: 1397997
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
 drivers/spi/spi-s3c64xx.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index b392cca..9f1013e 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -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;
 
-- 
2.5.0

Re: [PATCH] spi: add null check before pointer dereference

[Krzysztof Kozlowski]

On Tue, May 23, 2017 at 2:25 AM, Gustavo A. R. Silva
<garsilva@embeddedor.com> wrote:
> Add null check before dereferencing pointer desc
>
> Addresses-Coverity-ID: 1397997
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index b392cca..9f1013e 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -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

[PATCH] spi: add null check before pointer dereference

[Krzysztof Kozlowski]

On Tue, May 23, 2017 at 2:25 AM, Gustavo A. R. Silva
<garsilva@embeddedor.com> wrote:
> Add null check before dereferencing pointer desc
>
> Addresses-Coverity-ID: 1397997
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
>  drivers/spi/spi-s3c64xx.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index b392cca..9f1013e 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -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

Re: [PATCH] spi: add null check before pointer dereference

[Geert Uytterhoeven]

On Tue, May 23, 2017 at 9:24 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Tue, May 23, 2017 at 2:25 AM, Gustavo A. R. Silva
> <garsilva@embeddedor.com> wrote:
>> Add null check before dereferencing pointer desc
>>
>> Addresses-Coverity-ID: 1397997
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>>  drivers/spi/spi-s3c64xx.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index b392cca..9f1013e 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -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?

I.e. does it fall back to PIO?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@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

Re: [PATCH] spi: add null check before pointer dereference

[Geert Uytterhoeven]

On Tue, May 23, 2017 at 9:24 AM, Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, May 23, 2017 at 2:25 AM, Gustavo A. R. Silva
> <garsilva-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org> wrote:
>> Add null check before dereferencing pointer desc
>>
>> Addresses-Coverity-ID: 1397997
>> Signed-off-by: Gustavo A. R. Silva <garsilva-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
>> ---
>>  drivers/spi/spi-s3c64xx.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index b392cca..9f1013e 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -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?

I.e. does it fall back to PIO?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] spi: add null check before pointer dereference

[Geert Uytterhoeven]

On Tue, May 23, 2017 at 9:24 AM, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Tue, May 23, 2017 at 2:25 AM, Gustavo A. R. Silva
> <garsilva@embeddedor.com> wrote:
>> Add null check before dereferencing pointer desc
>>
>> Addresses-Coverity-ID: 1397997
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>>  drivers/spi/spi-s3c64xx.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index b392cca..9f1013e 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -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?

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

Re: [PATCH] spi: add null check before pointer dereference

[Andi Shyti]

Hi Gustavo,

>  	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

[PATCH] spi: add null check before pointer dereference

[Andi Shyti]

Hi Gustavo,

>  	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

Re: [PATCH] spi: add null check before pointer dereference

[Gustavo A. R. Silva]

Hi Andi,

Quoting Andi Shyti <andi.shyti@samsung.com>:

> Hi Gustavo,
>
>>  	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.
>

Yeah, I get it.

> That's anyway in my todo list.
>

That's great.

> Thanks,
> Andi

Thanks!
--
Gustavo A. R. Silva

Re: [PATCH] spi: add null check before pointer dereference

[Gustavo A. R. Silva]

Hi Andi,

Quoting Andi Shyti <andi.shyti@samsung.com>:

> Hi Gustavo,
>
>>  	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.
>

Yeah, I get it.

> That's anyway in my todo list.
>

That's great.

> Thanks,
> Andi

Thanks!
--
Gustavo A. R. Silva

[PATCH] spi: add null check before pointer dereference

[Gustavo A. R. Silva]

Hi Andi,

Quoting Andi Shyti <andi.shyti@samsung.com>:

> Hi Gustavo,
>
>>  	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.
>

Yeah, I get it.

> That's anyway in my todo list.
>

That's great.

> Thanks,
> Andi

Thanks!
--
Gustavo A. R. Silva