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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y39XFzYJL3EmxSFF@sirena.org.uk>
Date:   Thu, 24 Nov 2022 11:35:51 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Dhruva Gole <d-gole@...com>
Cc:     Nathan Barrett-Morrison <nathan.morrison@...esys.com>,
        greg.malysa@...esys.com,
        "open list:SPI SUBSYSTEM" <linux-spi@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] spi: cadence-quadspi: Add upper limit safety check to
 baudrate divisor

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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)