linux-kernel - Re: [PATCH] mfd: syscon: Set max_register_is_0 when syscon points to a single register

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: <ce44b268-d138-445d-a149-e5348897d3c5@sirena.org.uk>
Date: Wed, 28 Aug 2024 15:27:28 +0100
From: Mark Brown <broonie@...nel.org>
To: Nishanth Menon <nm@...com>
Cc: Arnd Bergmann <arnd@...db.de>, Lee Jones <lee@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	afd@...com, bb@...com, d-gole@...com,
	Jan Dakinevich <jan.dakinevich@...utedevices.com>
Subject: Re: [PATCH] mfd: syscon: Set max_register_is_0 when syscon points to
 a single register

On Wed, Aug 28, 2024 at 08:32:29AM -0500, Nishanth Menon wrote:
> On 13:57-20240828, Mark Brown wrote:
> > On Wed, Aug 28, 2024 at 07:10:08AM -0500, Nishanth Menon wrote:

> > > Fixes: 0ec74ad3c157 ("regmap: rework ->max_register handling")

> > In what sense is this a fix?

> The max_register was 0x0 was clearly a corner case. The fix done for
> remap  should have cleaned up the users of max_register to maintain the
> behavior. That is just my opinion.

No, apart from the fact that catching all possible regmap users would be
rather hard it's entirely optional for regmaps to specify a maxium
register.

> > really does not seem like a good idea - unless you've done an audit of
> > every single syscon to make sure they do explicitly specify a maximum
> > register, and confirmed that this can't be specified via DT, then it's
> > going to break things.

> I understand the risk - but having a consistent max_register definition
> is important - key here is that in regmap, max_register is valid if:
> a) max_register not being 0
> b) if max_register is 0, it is valid only if max_register_is_0 is set to
> true.

> When syscon sets the max_register, it operates correctly for num_reg > 1
> however, when reg_size == 1, you don't get the checks that you
> get when num_regs > 1. That is inconsistent behavior.

> It might help if you can clarify why you think an inconsistent behavior
> is correct for syscon?

Like I say specifying a maximum register is entirely optional, not
everyone wants that feature and if you don't use the debugfs interface
or the flat cache it doesn't *super* matter.  With 0 as default it's
always going to be awkward to describe a maximum register of 0 while
allowing that to be optional, fortunately very few devices have a single
register.

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