[v3,0/8] hwmon: (pmbus/adm1266) GPIO, NVMEM, and debugfs accessor fixes

Eight pre-existing bugs in the adm1266 driver's userspace-facing
accessors and probe ordering.  Each is reachable any time userspace
touches an ADM1266 GPIO/PDIO line via the gpiolib char-dev or sysfs
interfaces, opens the nvmem device, or reads the sequencer_state
debugfs entry.  Five landed when GPIO support was added (commit
d98dfad35c38), one when blackbox/NVMEM was added (commit
15609d189302), and one when the sequencer_state debugfs entry was
added (commit ed1ff457e187).

Patch 1 caps the PDIO scan loop in adm1266_gpio_get_multiple() at
ADM1266_PDIO_NR (16) instead of ADM1266_PDIO_STATUS (0xE9 = 233, a
PMBus command code that ended up in the bound by mistake).  As
written, the scan walks find_next_bit() up to bit 242 across a
25-bit caller mask, reading out of bounds and -- if any of that
incidental memory contains a set bit -- driving a corresponding
out-of-bounds write to the caller's bits array.

Patch 2 drops a redundant "*bits = 0" reset that sits between the
GPIO and PDIO halves of adm1266_gpio_get_multiple().  As written,
the GPIO bits the first loop populates are immediately discarded
before the PDIO loop runs, so any caller asking for a mix of GPIO
and PDIO lines sees the GPIO half always reported as 0.

Patch 3 adds the missing "ret < 2" length check after the three
i2c_smbus_read_block_data() calls in adm1266_gpio_get() and
adm1266_gpio_get_multiple().  A device returning a 0- or 1-byte
response would otherwise compose pin status from uninitialised
stack memory and leak it to userspace via gpiolib.

Patch 4 moves adm1266_config_gpio() past pmbus_do_probe() in
adm1266_probe() so the gpio_chip isn't registered (and reachable
from userspace) until the PMBus state the GPIO accessors depend
on is initialised.  This is a prerequisite for patch 6.

Patch 5 does the same for adm1266_config_nvmem(): the nvmem
device is now also registered after pmbus_do_probe(), so the
nvmem callback (adm1266_nvmem_read) cannot race
pmbus_do_probe()'s own device accesses.

Patch 6 takes pmbus_lock at the top of adm1266_gpio_get(),
adm1266_gpio_get_multiple(), and adm1266_gpio_dbg_show() so the
GPIO PMBus reads can't land between a PAGE write and the paged
read pmbus_core does in another thread.

Patch 7 takes pmbus_lock in adm1266_nvmem_read() so its memset /
blackbox refill / memcpy run as a single critical section.  The
NVMEM core does not serialise concurrent reg_read invocations, so
without this two readers racing at offset 0 can interleave the
memset on data->dev_mem with another reader's memcpy to userspace
and return torn data.  The same lock also serialises the refill's
PMBus traffic against pmbus_core's own PAGE+register sequences.

Patch 8 takes pmbus_lock in adm1266_state_read() (the
sequencer_state debugfs handler) for the same defensive-locking
reason: any direct device access from outside pmbus_core should
be ordered with respect to pmbus_core's own.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Changes in v3:

- New patch 5: register the nvmem device after pmbus_do_probe()
  in adm1266_probe().  Same probe-ordering hazard the v2 patch 4
  fixed for the gpio_chip, just for nvmem; flagged by Sashiko on
  v2 and seconded by Guenter on-list.

- Patches 1 and 2: pick up Linus Walleij's Reviewed-by from his
  reply to v1 [1].

- All previously-reviewed patches (1-4 of v2, now 1-4 of v3, plus
  6 which is the v2 patch 5 renumbered): pick up Bartosz
  Golaszewski's Reviewed-by from his reply to v2 [2].

- Patch 1: tighten the commit-message wording about how far the
  unbounded scan reads past the end of the caller mask.  v2 said
  "up to 27 extra unsigned-long words", but the actual extent is
  217 bits past gc.ngpio = 25, which works out to a handful of
  long-word reads (four on 64-bit, seven on 32-bit) -- Sashiko
  caught the arithmetic on v2.  No code change.

- New patch 7: take pmbus_lock in adm1266_nvmem_read() so the
  memset / blackbox refill / memcpy on data->dev_mem run as a
  single critical section, and so the refill's PMBus traffic
  serialises against pmbus_core's PAGE+register sequences.
  Depends on the new patch 5 for the probe-ordering invariant:
  without it, an early NVMEM read could fire before pmbus_do_probe()
  has wired up the pmbus_core data, and the lock acquisition would
  deref a NULL i2c_get_clientdata().  Flagged by Sashiko on v2.

- New patch 8: take pmbus_lock in adm1266_state_read() (the
  sequencer_state debugfs handler) for the same defensive-locking
  reason that motivates patch 6.  adm1266_init_debugfs() already
  runs after pmbus_do_probe() so no extra probe-ordering work is
  needed.  Flagged by Sashiko on v2.

[1] https://lore.kernel.org/r/CAD++jL=rasuYTot3M8u75PXRgrhbCzpue=pY2Yxx7ymVwhgGGQ@mail.gmail.com
[2] https://lore.kernel.org/r/CAMRc=Md_kjwH5v1JkQz5DxnzhK9yk1t+kjNcHG7P75bdg_16xA@mail.gmail.com
- Link to v2: https://patch.msgid.link/20260516-adm1266-gpio-fixes-v2-0-801f13debcb2@nexthop.ai

Changes in v2:
- New patch 3: reject short block-read responses in adm1266_gpio_get()
  and adm1266_gpio_get_multiple(), so a 0- or 1-byte response from
  the device cannot leak uninitialised stack memory to userspace
  through the gpiolib interfaces (Sashiko review of v1).
- New patch 4: move adm1266_config_gpio() down past pmbus_do_probe()
  in adm1266_probe() so the gpio_chip isn't reachable from userspace
  before the PMBus state the GPIO accessors depend on is initialised.
  Prerequisite for the new patch 5; without it the lock acquired by
  the GPIO accessors would race adm1266_config_gpio() / pmbus_do_probe()
  setup.
- New patch 5: take pmbus_lock in adm1266_gpio_get(),
  adm1266_gpio_get_multiple(), and adm1266_gpio_dbg_show() so the
  GPIO PMBus reads serialise against pmbus_core's PAGE+register
  sequences (Sashiko review of v1).
- Patches 1 and 2 are unchanged from v1.
- Link to v1: https://patch.msgid.link/20260516-adm1266-gpio-fixes-v1-0-38d9dd39b905@nexthop.ai

To: Guenter Roeck <linux@roeck-us.net>
To: Linus Walleij <linusw@kernel.org>
To: Bartosz Golaszewski <brgl@kernel.org>
To: Alexandru Tachici <alexandru.tachici@analog.com>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-gpio@vger.kernel.org

---
Abdurrahman Hussain (8):
      hwmon: (pmbus/adm1266) cap PDIO scan in get_multiple at ADM1266_PDIO_NR
      hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple
      hwmon: (pmbus/adm1266) reject short block-read responses in the GPIO accessors
      hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe()
      hwmon: (pmbus/adm1266) register the nvmem device after pmbus_do_probe()
      hwmon: (pmbus/adm1266) serialize GPIO PMBus accesses with pmbus_lock
      hwmon: (pmbus/adm1266) serialize NVMEM blackbox read with pmbus_lock
      hwmon: (pmbus/adm1266) serialize sequencer_state debugfs read with pmbus_lock

 drivers/hwmon/pmbus/adm1266.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)
---
base-commit: 70eda68668d1476b459b64e69b8f36659fa9dfa8
change-id: 20260516-adm1266-gpio-fixes-dbdb9c10a4c2

Best regards,
--  
Abdurrahman Hussain <abdurrahman@nexthop.ai>

On 5/18/26 17:52, Abdurrahman Hussain wrote:
> Eight pre-existing bugs in the adm1266 driver's userspace-facing
> accessors and probe ordering.  Each is reachable any time userspace
> touches an ADM1266 GPIO/PDIO line via the gpiolib char-dev or sysfs
> interfaces, opens the nvmem device, or reads the sequencer_state
> debugfs entry.  Five landed when GPIO support was added (commit
> d98dfad35c38), one when blackbox/NVMEM was added (commit
> 15609d189302), and one when the sequencer_state debugfs entry was
> added (commit ed1ff457e187).
> 
> Patch 1 caps the PDIO scan loop in adm1266_gpio_get_multiple() at
> ADM1266_PDIO_NR (16) instead of ADM1266_PDIO_STATUS (0xE9 = 233, a
> PMBus command code that ended up in the bound by mistake).  As
> written, the scan walks find_next_bit() up to bit 242 across a
> 25-bit caller mask, reading out of bounds and -- if any of that
> incidental memory contains a set bit -- driving a corresponding
> out-of-bounds write to the caller's bits array.
> 
> Patch 2 drops a redundant "*bits = 0" reset that sits between the
> GPIO and PDIO halves of adm1266_gpio_get_multiple().  As written,
> the GPIO bits the first loop populates are immediately discarded
> before the PDIO loop runs, so any caller asking for a mix of GPIO
> and PDIO lines sees the GPIO half always reported as 0.
> 
> Patch 3 adds the missing "ret < 2" length check after the three
> i2c_smbus_read_block_data() calls in adm1266_gpio_get() and
> adm1266_gpio_get_multiple().  A device returning a 0- or 1-byte
> response would otherwise compose pin status from uninitialised
> stack memory and leak it to userspace via gpiolib.
> 
> Patch 4 moves adm1266_config_gpio() past pmbus_do_probe() in
> adm1266_probe() so the gpio_chip isn't registered (and reachable
> from userspace) until the PMBus state the GPIO accessors depend
> on is initialised.  This is a prerequisite for patch 6.
> 
> Patch 5 does the same for adm1266_config_nvmem(): the nvmem
> device is now also registered after pmbus_do_probe(), so the
> nvmem callback (adm1266_nvmem_read) cannot race
> pmbus_do_probe()'s own device accesses.
> 
> Patch 6 takes pmbus_lock at the top of adm1266_gpio_get(),
> adm1266_gpio_get_multiple(), and adm1266_gpio_dbg_show() so the
> GPIO PMBus reads can't land between a PAGE write and the paged
> read pmbus_core does in another thread.
> 
> Patch 7 takes pmbus_lock in adm1266_nvmem_read() so its memset /
> blackbox refill / memcpy run as a single critical section.  The
> NVMEM core does not serialise concurrent reg_read invocations, so
> without this two readers racing at offset 0 can interleave the
> memset on data->dev_mem with another reader's memcpy to userspace
> and return torn data.  The same lock also serialises the refill's
> PMBus traffic against pmbus_core's own PAGE+register sequences.
> 
> Patch 8 takes pmbus_lock in adm1266_state_read() (the
> sequencer_state debugfs handler) for the same defensive-locking
> reason: any direct device access from outside pmbus_core should
> be ordered with respect to pmbus_core's own.
> 
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Series applied.

Thanks,
Guenter