Re: [PATCH] spi: cadence-quadspi: Add upper limit safety check to baudrate divisor

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1129 bytes --]

On Thu, Nov 24, 2022 at 12:16:10PM +0530, Dhruva Gole wrote:
> On 24/11/22 02:47, Nathan Barrett-Morrison wrote:

> > +	/* Maximum baud divisor */
> > +	if (div > CQSPI_REG_CONFIG_BAUD_MASK)

> I don't think comparing "greater than" with a MASK is atall a good idea.

Why - it's checking that the calculated divisor can actually fit in the
relevant register field which seems like a totally normal thing to do?

> Again, I don't fully understand your situation is as in
> what is the peripheral you are using. So please elaborate on that.

As far as I can tell the issue here is that the device is asking for a
rate which requires a larger divisor than the controller can support but
the driver doesn't do any bounds checking so it just writes the
calculated divisor out to the hardware, corrupting any adjacent fields.

In this context the SPI controller is a peripheral within the SoC.

> Importantly, I would suggest that you _NEVER_ compare ANY value to a
> MASK Macro. MASK Macros are meant to MASK bits.

It's very common to also use masks to identify when values have
overflowed the values that can be written to a field.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]