linux-kernel - Re: [RFC PATCH 1/2] gpio: regmap: Force writes for aliased data regs

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: <6bf0198d6e67a67e9f72fd27de86d65dc926d041.camel@svanheule.net>
Date: Mon, 20 Oct 2025 15:25:53 +0200
From: Sander Vanheule <sander@...nheule.net>
To: Michael Walle <mwalle@...nel.org>, Linus Walleij
 <linus.walleij@...aro.org>,  Bartosz Golaszewski	 <brgl@...ev.pl>,
 linux-gpio@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/2] gpio: regmap: Force writes for aliased data regs

Hi Michael,

On Mon, 2025-10-20 at 15:02 +0200, Michael Walle wrote:
> Hi Sander,
> 
> On Mon Oct 20, 2025 at 1:56 PM CEST, Sander Vanheule wrote:
> > GPIO chips often have data input and output fields aliased to the same
> > offset. Since gpio-regmap performs a value update before the direction
> > update (to prevent glitches), a pin currently configured as input may
> > cause regmap_update_bits() to not perform a write.
> > 
> > This may cause unexpected line states when the current input state
> > equals the requested output state:
> > 
> >         OUT   IN      OUT
> >     DIR ''''''\...|.../''''''
> > 
> >     pin ....../'''|'''\......
> >              (1) (2) (3)
> > 
> >     1. Line was configurad as out-low, but is reconfigured to input.
> >        External logic results in high value.
> >     2. Set output value high. regmap_update_bits() sees the value is
> >        already high and discards the register write.
> >     3. Line is switched to output, maintaining the stale output config
> >        (low) instead of the requested config (high).
> > 
> > By switching to regmap_write_bits(), a write of the requested output
> > value can be forced, irrespective of the read state. Do this only for
> > aliased registers, so the more efficient regmap_update_bits() can still
> > be used for distinct registers.
> 
> Have you looked at the .volatile_reg callback of the regmap api?
> You might use the same heuristics, i.e. .reg_dat_base == .reg_set_base
> to implement that callback. That way you'd just have to
> (unconditionally) set that callback in gpio_regmap_register() and
> regmap should take care of the rest.

Maybe I'm missing something here, but I'm not sure what difference that would
make. .volatile_reg is part of the regmap config, so when gpio_regmap_register()
is called, the regmap has already been created. We can't change the
.volatile_reg callback (and we shouldn't, it's up to the user to define it).

FWIW, I did test this with a regmap config that marks the aliased data registers
as volatile. The issue isn't that an invalid cache is being read. The problem is
that writes are being optimized away when they shouldn't:

   1. Read register from hardware (volatile) or cache (non-volatile).
   2. Update bits in mask to requested value
   3. Write updated value to hardware if:
         A. This is a forced write (i.e. regmap_write_bits), or
         B. The updated value differs from the original.

Marking the register as volatile doesn't change the behavior, only the source of
the initial value _regmap_update_bits() uses. Step 3B is the problematic one
when using regmap_update_bits(). Per the diagram above, the comparison may
happen against an input value differing from the (invisible) output state, which
would hide the state change.

Best,
Sander