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] [day] [month] [year] [list]

Message-ID: <Y39mkl+9W8S6ZzOk@sirena.org.uk>
Date:   Thu, 24 Nov 2022 12:41:54 +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 05:57:10PM +0530, Dhruva Gole wrote:
> On 24/11/22 17:05, Mark Brown wrote:

> > 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.

> but, I am not sure it would anyway corrupt any adjacent bits,

> The code
> reg |= (div & CQSPI_REG_CONFIG_BAUD_MASK) << CQSPI_REG_CONFIG_BAUD_LSB

> does mask the div value to ensure bits ONLY in CQSPI_REG_CONFIG_BAUD_MASK
> region are set and nothing else right?

Yes, that'd avoid corrupting adjacent bits (though it'd still be making
things worse in that it makes the divider smaller).

> I believe a simple warning is enough, and better not touch the div variable
> because it seems unnecessary. We already have a mask to take care of masking
> the appropriate bits.

That'd still leave the clock driven too fast which could break things,
going for the maximum divider would mitigate this (though an error would
be even safer).

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