linux-kernel - Re: [PATCH] regmap: fix writes to non incrementing registers

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: <20190903121210.GA7916@sirena.co.uk>
Date:   Tue, 3 Sep 2019 13:12:10 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Ben Whitten <ben.whitten@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andreas Färber <afaerber@...e.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>, nandor.han@...sala.com
Subject: Re: [PATCH] regmap: fix writes to non incrementing registers

On Tue, Sep 03, 2019 at 10:42:59AM +0100, Ben Whitten wrote:

> You mentioned that we likely have breakage elsewhere, I believe that
> regmap_noinc_write probably shouldn't ever have been calling
> _regmap_raw_write.

> Whilst regmap_noinc_read calls _regmap_raw_read, this function doesn't
> do any massage and just calls map->bus->read after selecting a page.
> regmap_raw_write on the other hand is meant for writing blocks to
> register ranges as these added checks show, and does split work based
> on page length etc.
> This splitting up is something that shouldn't apply to the FIFO as it's a
> deep register.

This just means that we need to take care of nonincrementing registers
in there, we don't want to end up with too many different places where
we might have to handle formatting since that leads to duplication.
It's about marshalling for physical I/O, it's a bit misnamed at this
point but nonincrementing registers definitely fit in there for me.

> I modified regmap_noinc_write to be much more like the raw_read
> internals, just select page then attempt a map->bus->gather_write,
> falling back to linearising if that's not supported.
> This approach worked at getting writing working into the FIFO.

Modify raw_write() to handle this instead, it is a mirror of the read
side it's just that writes are more complicated since there's more
things that could happen as you're discovering here.

> So I would propose that there are two 'Fixes:' patches here, one
> enhancing the checking when writing a register range to ensure you
> don't stumble into a noinc register.

Yes.

> And one to fix noinc_writes to send data directly to the bus once the
> page is selected with no splitting up as you would a range.

Push that into raw_write(), it just needs to special case when the first
register in a block is non-incrementing in the page selection logic (add
a !nonincrmenting in the while loop I think).

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