[PATCH 0/2] hwmon: (pmbus/adm1266) adm1266_gpio_get_multiple() fixes

Abdurrahman Hussain posted 2 patches 1 week, 1 day ago
drivers/hwmon/pmbus/adm1266.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH 0/2] hwmon: (pmbus/adm1266) adm1266_gpio_get_multiple() fixes
Posted by Abdurrahman Hussain 1 week, 1 day ago
Two pre-existing bugs in adm1266_gpio_get_multiple() that landed
together when GPIO support was first added (commit d98dfad35c38).
Both are reachable any time userspace queries multiple ADM1266 GPIO
or PDIO lines at once via the gpiolib char-dev or sysfs interfaces.

Patch 1 caps the PDIO scan loop 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.  Flagged by sashiko in review of an unrelated
fix series [1].

Patch 2 drops a redundant "*bits = 0" reset that sits between the
GPIO and PDIO halves of the function.  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.

[1] https://sashiko.dev/#/patchset/20260515-adm1266-fixes-v1-0-1c1ea1349cfe@nexthop.ai

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Abdurrahman Hussain (2):
      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

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

Best regards,
--  
Abdurrahman Hussain <abdurrahman@nexthop.ai>
Re: [PATCH 0/2] hwmon: (pmbus/adm1266) adm1266_gpio_get_multiple() fixes
Posted by Linus Walleij 1 week ago
On Sat, May 16, 2026 at 10:45 PM Abdurrahman Hussain
<abdurrahman@nexthop.ai> wrote:

> Two pre-existing bugs in adm1266_gpio_get_multiple() that landed
> together when GPIO support was first added (commit d98dfad35c38).
> Both are reachable any time userspace queries multiple ADM1266 GPIO
> or PDIO lines at once via the gpiolib char-dev or sysfs interfaces.
>
> Patch 1 caps the PDIO scan loop 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.  Flagged by sashiko in review of an unrelated
> fix series [1].
>
> Patch 2 drops a redundant "*bits = 0" reset that sits between the
> GPIO and PDIO halves of the function.  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.
>
> [1] https://sashiko.dev/#/patchset/20260515-adm1266-fixes-v1-0-1c1ea1349cfe@nexthop.ai
>
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>

Reviewed-by: Linus Walleij <linusw@kernel.org>

The better option would be to:

1. Convert this driver to use regmap
2. Extend gpio-regmap with get/set_multiple()
3. Convert the driver to use gpio-regmap

So if you feel adventurous and have time on your hands,
consider it! :)

Yours,
Linus Walleij
Re: [PATCH 0/2] hwmon: (pmbus/adm1266) adm1266_gpio_get_multiple() fixes
Posted by Guenter Roeck 6 days, 8 hours ago
On 5/17/26 03:44, Linus Walleij wrote:
> On Sat, May 16, 2026 at 10:45 PM Abdurrahman Hussain
> <abdurrahman@nexthop.ai> wrote:
> 
>> Two pre-existing bugs in adm1266_gpio_get_multiple() that landed
>> together when GPIO support was first added (commit d98dfad35c38).
>> Both are reachable any time userspace queries multiple ADM1266 GPIO
>> or PDIO lines at once via the gpiolib char-dev or sysfs interfaces.
>>
>> Patch 1 caps the PDIO scan loop 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.  Flagged by sashiko in review of an unrelated
>> fix series [1].
>>
>> Patch 2 drops a redundant "*bits = 0" reset that sits between the
>> GPIO and PDIO halves of the function.  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.
>>
>> [1] https://sashiko.dev/#/patchset/20260515-adm1266-fixes-v1-0-1c1ea1349cfe@nexthop.ai
>>
>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
> 
> Reviewed-by: Linus Walleij <linusw@kernel.org>
> 
> The better option would be to:
> 
> 1. Convert this driver to use regmap

That would mean to convert the pmbus core code to regmap,
plus all the pmbus client drivers.

PMBus uses a mix of registers/command with different size, plus some
block commands. That would be a difficult task. Byte registers can be
mapped to word size, but for block registers that is difficult,
and then there are commands with zero data length. Maybe someone
managed to do this somewhere. I tried some time ago and could not get
it to work.

Guenter

> 2. Extend gpio-regmap with get/set_multiple()
> 3. Convert the driver to use gpio-regmap
> 
> So if you feel adventurous and have time on your hands,
> consider it! :)
> 
> Yours,
> Linus Walleij
>