linux-kernel - RE: [spi-devel-general] [PATCH 1/2] spi: new SPI bus lock/unlockfunctions

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]

Date:	Fri, 18 Sep 2009 19:00:13 -0400
From:	"H Hartley Sweeten" <hartleys@...ionengravers.com>
To:	"Mike Frysinger" <vapier.adi@...il.com>
Cc:	<spi-devel-general@...ts.sourceforge.net>,
	"David Brownell" <dbrownell@...rs.sourceforge.net>,
	"Yi Li" <yi.li@...log.com>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	<linux-kernel@...r.kernel.org>
Subject: RE: [spi-devel-general] [PATCH 1/2] spi: new SPI bus lock/unlockfunctions

On Thursday, September 17, 2009 3:54 PM, Mike Frysinger wrote:
>> I assume the spi master driver must supply the {lock/unlock}_bus methods
>> to properly support the locking.
>
> currently, yes.  a common solution would be nice.  ideas/patches welcome ;).
>
>> But, by returning 0 when the methods
>> are not supplied you are basically saying all the current master drivers
>> in mainline support bus locking.  I think this is really only "true" if
>> spi->master->num_chipselect == 1.
>
> right, but that is no different from what we have today.  there is no
> way for a client to guarantee exclusive access, so you cant write code
> assuming it in the first place.  the only consumer thus far (mmc_spi)
> actually errors out if such conditions exist.
>
> in other words, we arent breaking anything.

Actually you are breaking the mmc_spi driver.

By returning 0 when the methods are not supplied you are saying that the
master driver supports and locked the bus.  At a minimum, I think spi_lock_bus()
should return an error code if locking is not supported.

Also, as Andrew Morton pointed out, calling spi_unlock_bus() without having
a valid lock by calling spi_lock_bus() is a bug.

In addition your patch to mmc_spi should check the return code from
spi_lock_bus().  If the driver "requires" that the bus be locked it should
trigger an error path if it cannot be locked.

>> Also, do you have a master driver that does have the {lock/unlock}_bus
>> methods?  I would like to see how you handled it.
>
> http://git.kernel.org/?p=linux/kernel/git/vapier/blackfin.git;a=commitdiff;h=cc54fa8ed63e11a000031bc650d41d57b441803d

Oiy... The lock/unlock functions are simple enough but the change needed
to bfin_spi_pump_messages() is a bit complicated.

What happens to next_msg if it is for other devices on the bus?

Regards,
Hartley