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

[Guenter Roeck]

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@xxxxxxxxxx>

Series applied.

Thanks,
Guenter