2014-02-13 14:48:18

by Ivan T. Ivanov

[permalink] [raw]
Subject: [PATCH] spi: core: Validate lenght of the transfers in message

From: "Ivan T. Ivanov" <[email protected]>

SPI transfer lenght should be a power-of-two multiple
of eight bits.

Signed-off-by: Ivan T. Ivanov <[email protected]>
---
drivers/spi/spi.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 23756b0..474c0b0 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1619,6 +1619,7 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
{
struct spi_master *master = spi->master;
struct spi_transfer *xfer;
+ int w_size, n_words;

if (list_empty(&message->transfers))
return -EINVAL;
@@ -1670,6 +1671,22 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
return -EINVAL;
}

+ /*
+ * Word size of the SPI transfer should be a power-of-two
+ * multiple of eight bits,
+ */
+ if (xfer->bits_per_word <= 8)
+ w_size = 1;
+ else if (xfer->bits_per_word <= 16)
+ w_size = 2;
+ else
+ w_size = 4;
+
+ n_words = xfer->len / w_size;
+ /* No partial transfers accepted */
+ if (!n_words || xfer->len & (w_size - 1))
+ return -EINVAL;
+
if (xfer->speed_hz && master->min_speed_hz &&
xfer->speed_hz < master->min_speed_hz)
return -EINVAL;
--
1.7.9.5


2014-02-13 21:28:02

by Gerhard Sittig

[permalink] [raw]
Subject: Re: [PATCH] spi: core: Validate lenght of the transfers in message

On Thu, Feb 13, 2014 at 16:46 +0200, Ivan T. Ivanov wrote:
>
> From: "Ivan T. Ivanov" <[email protected]>
>
> SPI transfer lenght should be a power-of-two multiple
> of eight bits.

Are you suggesting that an SPI transfer cannot consist of e.g.
three bytes? This would be surprising, and certainly would be
rather wrong an assumption. Am I missing/misunderstanding
something?


virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: [email protected]

2014-02-14 00:26:39

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] spi: core: Validate lenght of the transfers in message

On Thu, Feb 13, 2014 at 10:27:57PM +0100, Gerhard Sittig wrote:
> On Thu, Feb 13, 2014 at 16:46 +0200, Ivan T. Ivanov wrote:

> > SPI transfer lenght should be a power-of-two multiple
> > of eight bits.

> Are you suggesting that an SPI transfer cannot consist of e.g.
> three bytes? This would be surprising, and certainly would be
> rather wrong an assumption. Am I missing/misunderstanding
> something?

The check is supposed to be verifying that the transfer is a whole
number of the current SPI word size as I understand it (not actually
looked at the change yet but that's where it came from, factored out of
a driver).


Attachments:
(No filename) (628.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-02-14 08:17:30

by Ivan T. Ivanov

[permalink] [raw]
Subject: Re: [PATCH] spi: core: Validate lenght of the transfers in message

On Fri, 2014-02-14 at 00:26 +0000, Mark Brown wrote:
> On Thu, Feb 13, 2014 at 10:27:57PM +0100, Gerhard Sittig wrote:
> > On Thu, Feb 13, 2014 at 16:46 +0200, Ivan T. Ivanov wrote:
>
> > > SPI transfer lenght should be a power-of-two multiple
> > > of eight bits.
>
> > Are you suggesting that an SPI transfer cannot consist of e.g.
> > three bytes? This would be surprising, and certainly would be
> > rather wrong an assumption. Am I missing/misunderstanding
> > something?
>
> The check is supposed to be verifying that the transfer is a whole
> number of the current SPI word size as I understand it (not actually
> looked at the change yet but that's where it came from, factored out of
> a driver).

Yes. Comment is not correct. Is this better?

"SPI transfer length should be multiple of SPI word size, where SPI
word size should be power-of-two multiple"

Regards,
Ivan

2014-02-14 18:26:21

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] spi: core: Validate lenght of the transfers in message

On Thu, Feb 13, 2014 at 04:46:46PM +0200, Ivan T. Ivanov wrote:

> + /* No partial transfers accepted */
> + if (!n_words || xfer->len & (w_size - 1))
> + return -EINVAL;

Please write this using % rather than the & - it's a lot clearer what
it's checking for, I had to think what the above meant. Hopefully the
compiler will make it equally fast either way.


Attachments:
(No filename) (364.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-02-14 18:26:50

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] spi: core: Validate lenght of the transfers in message

On Fri, Feb 14, 2014 at 10:16:05AM +0200, Ivan T. Ivanov wrote:

> "SPI transfer length should be multiple of SPI word size, where SPI
> word size should be power-of-two multiple"

Yes, that's clearer though you could in theory have a three byte word
(I'm not sure that anyone would actually do that but it's possible). I
do have a comment on the code as well.


Attachments:
(No filename) (362.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments