[1/7] spi: core: Validate Tx/Rx buffers for non-zero length transfer
diff mbox

Message ID 1395201227.16346.1.camel@phoenix
State New, archived
Headers show

Commit Message

Axel Lin March 19, 2014, 3:53 a.m. UTC
Reject transfer if both tx_buf and rx_buf are NULL for non-zero length transfer.
Add the checking in spi core so we can remove the same checking in various
drivers.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/spi/spi.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Geert Uytterhoeven March 19, 2014, 8:06 a.m. UTC | #1
On Wed, Mar 19, 2014 at 4:53 AM, Axel Lin <axel.lin@ingics.com> wrote:
> Reject transfer if both tx_buf and rx_buf are NULL for non-zero length transfer.
> Add the checking in spi core so we can remove the same checking in various
> drivers.

Just wondering: are there any SPI slave drivers that implement dummy cycles
this way?

> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/spi/spi.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 26b331d..0a29d2a 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1876,6 +1876,10 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
>                 if (xfer->len % w_size)
>                         return -EINVAL;
>
> +               /* non-zero length transfer must has tx_buf or rx_buf */

have

> +               if (xfer->len && !xfer->tx_buf && !xfer->rx_buf)
> +                       return -EINVAL;
> +

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
--
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
Axel Lin March 19, 2014, 3:43 p.m. UTC | #2
2014-03-19 16:06 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:
> On Wed, Mar 19, 2014 at 4:53 AM, Axel Lin <axel.lin@ingics.com> wrote:
>> Reject transfer if both tx_buf and rx_buf are NULL for non-zero length transfer.
>> Add the checking in spi core so we can remove the same checking in various
>> drivers.
>
> Just wondering: are there any SPI slave drivers that implement dummy cycles
> this way?

I think I found one such case:
drivers/net/wireless/libertas/if_spi.c
the dummy_trans has non-zero length, and both tx_buf and rx_buf are NULL.

But that means it doesn't work with master drivers with this checking, e.g.
all drivers using bitbang library has this checking.

Regards,
Axel
--
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
Mark Brown March 19, 2014, 4:08 p.m. UTC | #3
On Wed, Mar 19, 2014 at 11:43:55PM +0800, Axel Lin wrote:
> 2014-03-19 16:06 GMT+08:00 Geert Uytterhoeven <geert@linux-m68k.org>:

> > Just wondering: are there any SPI slave drivers that implement dummy cycles
> > this way?

> I think I found one such case:
> drivers/net/wireless/libertas/if_spi.c
> the dummy_trans has non-zero length, and both tx_buf and rx_buf are NULL.

> But that means it doesn't work with master drivers with this checking, e.g.
> all drivers using bitbang library has this checking.

I have to say that this seems rather abusive on the part of the driver,
it's going to cause implementation issues in the masters - I expect more
than just those that check aren't doing to do the right thing.
Mark Brown Aug. 16, 2014, 2:34 p.m. UTC | #4
On Wed, Mar 19, 2014 at 09:06:31AM +0100, Geert Uytterhoeven wrote:
> On Wed, Mar 19, 2014 at 4:53 AM, Axel Lin <axel.lin@ingics.com> wrote:

> > Reject transfer if both tx_buf and rx_buf are NULL for non-zero length transfer.
> > Add the checking in spi core so we can remove the same checking in various
> > drivers.

> Just wondering: are there any SPI slave drivers that implement dummy cycles
> this way?

The ChromeOS EC apparently does this.

Patch
diff mbox

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 26b331d..0a29d2a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1876,6 +1876,10 @@  static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 		if (xfer->len % w_size)
 			return -EINVAL;
 
+		/* non-zero length transfer must has tx_buf or rx_buf */
+		if (xfer->len && !xfer->tx_buf && !xfer->rx_buf)
+			return -EINVAL;
+
 		if (xfer->speed_hz && master->min_speed_hz &&
 		    xfer->speed_hz < master->min_speed_hz)
 			return -EINVAL;