linux-kernel - Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi

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: <20131211005106.GL11468@sirena.org.uk>
Date:	Wed, 11 Dec 2013 00:51:06 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	Samuel Ortiz <sameo@...ux.intel.com>,
	Lee Jones <lee.jones@...aro.org>,
	Srinivas Ramana <sramana@...eaurora.org>,
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 3/8] regmap: Add support for using regmap over ssbi

On Tue, Dec 10, 2013 at 04:13:15PM -0800, Stephen Boyd wrote:
> On 12/10/13 15:50, Mark Brown wrote:
> > On Tue, Dec 10, 2013 at 03:35:18PM -0800, Stephen Boyd wrote:

> >> +		reg += sizeof(u16);

> > I'd expect this to generate out of bounds accesses and bad interactions
> > on the bus if we go through the loop more than once since it appears to
> > incrementing the address of reg for every register.  I'm also having a

> ssbi_read() just reads the same register x number of times and doesn't
> do any sort of incrementing of address. My understanding was that
> regmap_bulk_read() will read incrementing addresses and then call down
> into this code with the sequential addresses formatted into the reg
> buffer. That was the flaw. Instead we need to take reg and then

That's possibly not the ideal decision for the underlying API, if
nothing else it's confusing naming given what a read function would
normally do.

> increment reg by 1 every time through this loop. Or should we just have
> use_single_rw == true?

No, it doesn't - it increments the address of reg by the size of a
register value each time.  Using use_single_rw might make sense, or if
you can't do bulk I/O at all then hooking in via reg_read() and
reg_write() in the config rather than trying to parse out the buffers
might be even better (you can still make helpers to set that up).

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