| Message ID | 1495101672-3384-1-git-send-email-jiada_wang@mentor.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 179547e143e773f9f866ad3536275ab627711f3a |
| Headers | show |
On Thu, 2017-05-18 at 03:01 -0700, jiada_wang@mentor.com wrote: > From: Jiada Wang <jiada_wang@mentor.com> > > In case either transfer->tx_buf or transfer->rx_buf is NULL, > manipulation of buffer in spi_imx_u32_swap_u[8|16]() will cause > NULL pointer dereference crash. > > Add buffer check at very beginning of spi_imx_u32_swap_u[8|16](), > to avoid such crash. > > Signed-off-by: Jiada Wang <jiada_wang@mentor.com> > Reported-by: Leonard Crestez <leonard.crestez@nxp.com> Tested-by: Leonard Crestez <leonard.crestez@nxp.com> -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thursday, May 18, 2017 06:01 PM, jiada_wang@mentor.com wrote: > From: Jiada Wang <jiada_wang@mentor.com> > > In case either transfer->tx_buf or transfer->rx_buf is NULL, > manipulation of buffer in spi_imx_u32_swap_u[8|16]() will cause > NULL pointer dereference crash. > > Add buffer check at very beginning of spi_imx_u32_swap_u[8|16](), > to avoid such crash. > > Signed-off-by: Jiada Wang <jiada_wang@mentor.com> > Reported-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > drivers/spi/spi-imx.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index 782045f..19b30cf 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -288,6 +288,9 @@ static void spi_imx_u32_swap_u8(struct spi_transfer *transfer, u32 *buf) > { > int i; > > + if (!buf) > + return; > + > for (i = 0; i < transfer->len / 4; i++) > *(buf + i) = cpu_to_be32(*(buf + i)); > } > @@ -296,6 +299,9 @@ static void spi_imx_u32_swap_u16(struct spi_transfer *transfer, u32 *buf) > { > int i; > > + if (!buf) > + return; > + > for (i = 0; i < transfer->len / 4; i++) { > u16 *temp = (u16 *)buf; > > Hi, thanks for the patch. But I think we missing something here. We return from a void function() so the error keeps hidden. The root cause is calling this functions with a NULL pointer. See if you can fix this by find the caller and check if the parameter hand over are valid. Cheers Chris
On Fri, May 19, 2017 at 3:45 PM, Chris Ruehl <chris.ruehl@gtsys.com.hk> wrote: > On Thursday, May 18, 2017 06:01 PM, jiada_wang@mentor.com wrote: > But I think we missing something here. We return from a void function() > so the error keeps hidden. The root cause is calling this functions with a > NULL pointer. See if you can fix this by find the caller and check if the > parameter hand over are valid. AFAIU some SPI controllers can have half-duplex protocol, in which the second buffer might be absent. > Disclaimer: http://www.gtsys.com.hk/email/classified.html Just wondering if it's okay to have this type of footer in open source community's mailing lists.
On Fri, May 19, 2017 at 04:15:01PM +0300, Andy Shevchenko wrote: > On Fri, May 19, 2017 at 3:45 PM, Chris Ruehl <chris.ruehl@gtsys.com.hk> wrote: > > But I think we missing something here. We return from a void function() > > so the error keeps hidden. The root cause is calling this functions with a > > NULL pointer. See if you can fix this by find the caller and check if the > > parameter hand over are valid. > AFAIU some SPI controllers can have half-duplex protocol, in which the > second buffer might be absent. Yes, it's not only perfectly valid but actually the common case for only one direction of data to be meaningful in SPI so the majority of controllers don't require data to be transferred in both directions, it would just add bus overhead.
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index 782045f..19b30cf 100644 --- a/drivers/spi/spi-imx.c +++ b/drivers/spi/spi-imx.c @@ -288,6 +288,9 @@ static void spi_imx_u32_swap_u8(struct spi_transfer *transfer, u32 *buf) { int i; + if (!buf) + return; + for (i = 0; i < transfer->len / 4; i++) *(buf + i) = cpu_to_be32(*(buf + i)); } @@ -296,6 +299,9 @@ static void spi_imx_u32_swap_u16(struct spi_transfer *transfer, u32 *buf) { int i; + if (!buf) + return; + for (i = 0; i < transfer->len / 4; i++) { u16 *temp = (u16 *)buf;