On Fri, 2015-04-17 at 17:02 +0100, Ian Abbott wrote:
> commit f20fbaad7620 ("spi: spidev: fix possible arithmetic overflow for multi-transfer message")
Queued up for 3.2, thanks.
Ben.
> `spidev_message()` sums the lengths of the individual SPI transfers to
> determine the overall SPI message length. It restricts the total
> length, returning an error if too long, but it does not check for
> arithmetic overflow. For example, if the SPI message consisted of two
> transfers and the first has a length of 10 and the second has a length
> of (__u32)(-1), the total length would be seen as 9, even though the
> second transfer is actually very long. If the second transfer specifies
> a null `rx_buf` and a non-null `tx_buf`, the `copy_from_user()` could
> overrun the spidev's pre-allocated tx buffer before it reaches an
> invalid user memory address. Fix it by checking that neither the total
> nor the individual transfer lengths exceed the maximum allowed value.
>
> Thanks to Dan Carpenter for reporting the potential integer overflow.
>
> Signed-off-by: Ian Abbott <abbotti@xxxxxxxxx>
> ---
> Note: original commit compares the lengths to INT_MAX instead of bufsiz
> due to changes in earlier commits.
> ---
> drivers/spi/spidev.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index 4eb7a98..7bf5186 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -245,7 +245,10 @@ static int spidev_message(struct spidev_data *spidev,
> k_tmp->len = u_tmp->len;
>
> total += k_tmp->len;
> - if (total > bufsiz) {
> + /* Check total length of transfers. Also check each
> + * transfer length to avoid arithmetic overflow.
> + */
> + if (total > bufsiz || k_tmp->len > bufsiz) {
> status = -EMSGSIZE;
> goto done;
> }
--
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.
Attachment:
signature.asc
Description: This is a digitally signed message part