linux-kernel - Re: [PATCH 2/2] spi: fix the divide by 0 error when calculating xfer waiting time

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20201229131308.GE4786@sirena.org.uk>
Date:   Tue, 29 Dec 2020 13:13:08 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Xu Yilun <yilun.xu@...el.com>
Cc:     linux-spi@...r.kernel.org, trix@...hat.com, lgoncalv@...hat.com,
        hao.wu@...el.com, matthew.gerlach@...ux.intel.com,
        russell.h.weight@...el.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] spi: fix the divide by 0 error when calculating xfer
 waiting time

On Tue, Dec 29, 2020 at 01:27:42PM +0800, Xu Yilun wrote:
> The xfer waiting time is the result of xfer->len / xfer->speed_hz, but
> when the following patch is merged,
> 
> commit 9326e4f1e5dd ("spi: Limit the spi device max speed to controller's max speed")
> 
> the xfer->speed_hz may always be clamped to 0 if the controller doesn't
> provide its max_speed_hz. There may be no hardware indication of the
> max_speed_hz so the controller driver leaves it, but exception happens
> when it tries to do irq mode transfer.

Does this still apply with current code?  There have been some fixes in
this area which I think should ensure that we don't turn the speed down
to 0 if the controller doesn't supply a limit IIRC.

> This patch makes the assumption of 1khz xfer speed if the xfer->speed_hz
> is not assigned. This avoids the divide by 0 issue and ensures a
> reasonable tolerant waiting time.

It will cause absurdly slow transfers if the controller does actually
implement speed setting though, if we're going to pick a default value
I'd go for at least 100kHz.  

>  	} else {
> +		speed_hz = xfer->speed_hz ? : 1000;

Please don't abuse the ternery operator, write normal conditional
statements to make things more legible.

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