linux-kernel - Re: [PATCH] spi: Limit the spi device max speed to controller's max speed

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: <CAMuHMdU+heMQKLZR15g5s5Ad-H8cDeFeM+7Wh=45PFqQhyfjOA@mail.gmail.com>
Date:   Tue, 15 Dec 2020 15:24:04 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Mark Brown <broonie@...nel.org>
Cc:     Tudor Ambarus <tudor.ambarus@...rochip.com>,
        linux-spi <linux-spi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] spi: Limit the spi device max speed to controller's max speed

Hi Mark, Tudor,

On Fri, Dec 11, 2020 at 8:02 PM Mark Brown <broonie@...nel.org> wrote:
> On Wed, 9 Dec 2020 19:35:14 +0200, Tudor Ambarus wrote:
> > Make sure the max_speed_hz of spi_device does not override
> > the max_speed_hz of controller.
>
> Applied to
>
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
>
> Thanks!
>
> [1/1] spi: Limit the spi device max speed to controller's max speed
>       commit: 9326e4f1e5dd1a4410c429638d3c412b6fc17040

> -       if (!spi->max_speed_hz)
> +       if (!spi->max_speed_hz ||
> +           spi->max_speed_hz > spi->controller->max_speed_hz)
>                 spi->max_speed_hz = spi->controller->max_speed_hz;

If spi->controller->max_speed_hz is zero, a non-zero spi->max_speed_hz
will be overwritten by zero.

Hence this broke spi-sh-msiof, which has the following check in
sh_msiof_spi_set_clk_regs():

        if (!spi_hz || !parent_rate) {
                WARN(1, "Invalid clock rate parameters %lu and %u\n",
                     parent_rate, spi_hz);
                return;
        }

Without this, the driver would trigger a division-by-zero later...

Arguably all SPI controller drivers should fill in
spi_controller.{min,max}_speed_hz, but as long as that is not the case,
I think this patch should be reverted, or the check should be enhanced
to make sure spi->controller->max_speed_hz is non-zero.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds