Re: [PATCH v2 1/2] sc16is7xx: Prevent TX buffer overrun, prevent crash

[Jakub Kiciński <moorray3@xxxxx>]

On Wed, 30 Sep 2015 16:09:44 +0200, Florian Achleitner wrote:
> If the chip wrongly reports a TX FIFO space, bigger than the driver's
> buffer, it runs over and destroys the struct sc16is7xx_port, its
> struct kworker, and very likely a lot more.
> For us, this lead to the immediate crash of the driver's kworker thread.
> 
> Prevent a buffer overrun by adding a length check.
> 
> Signed-off-by: Florian Achleitner <achleitner.florian@xxxxxxxxxxx>

Hi Florian!

Thanks a lot for the submission.  Would you mind digging a little
deeper into this problem?  I think the root cause is that the SPI
read fails for some reason and our driver does not handle that
properly, i.e. we don't read 255, it's just a random/failure value.
Unfortunatelly I don't have any boards with this chip any more to
do any tests or development myself, so it's on you ;)

This driver initially supported only I2C and in I2C regmap code if
the read fails we will always get a zero value.  Therefore we felt
free to ignore in sc16is7xx_port_read() the return value of
regmap_read().

Could you please run your tests again and see if perhaps the read is
failing?  In that case we should zero the return value from
sc16is7xx_port_read().

I think from this thread: 
https://lkml.org/lkml/2008/2/20/271
we can assume that zero-length writes are a valid use-case for SPI 
and if so could you please test the driver for your SPI controller?
Perhaps the zero-length check should be placed in the SPI controller
driver?

I am OK with adding the sanity checks but lets first get to the bottom
of this!

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html