Re: [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes

["Abdurrahman Hussain" <abdurrahman@nexthop.ai>]

On Mon May 18, 2026 at 3:08 PM PDT, Guenter Roeck wrote:
> Hi,
>
> On 5/16/26 16:18, Abdurrahman Hussain wrote:
>> Five pre-existing bugs in the adm1266 GPIO path that all landed when
>> GPIO support was first added (commit d98dfad35c38).  Each is
>> reachable any time userspace queries an ADM1266 GPIO/PDIO line via
>> the gpiolib char-dev or sysfs interfaces, or reads
>> debugfs/gpio-<chip>.
>> 
>> 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 5.
>> 
>> Patch 5 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.
>> 
>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> Sashiko reported a number of additional problems. As far as I can
> see those are real. Would you mind fixing those issues as well
> as part of this series ?
>
> Thanks,
> Guenter

Sure -- v3 (sending shortly) folds in everything Sashiko flagged on
v2 that isn't already covered by the "buffer-bound and timestamp
fixes" series you applied to hwmon-next:

  - New patch 5: register the nvmem device after pmbus_do_probe();
    same probe-ordering hazard v2 patch 4 fixed for the gpio_chip.
  - New patch 7: take pmbus_lock in adm1266_nvmem_read().
  - New patch 8: take pmbus_lock in adm1266_state_read().
  - Patch 1 commit-message wording fix (Sashiko corrected the
    "27 unsigned-long words" arithmetic; no code change).
  - Reviewed-by tags from Linus Walleij (patches 1, 2) and
    Bartosz Golaszewski (the rest).

Thanks,
Abdurrahman