* [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes
@ 2026-05-16 23:18 Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 1/5] hwmon: (pmbus/adm1266) cap PDIO scan in get_multiple at ADM1266_PDIO_NR Abdurrahman Hussain
` (6 more replies)
0 siblings, 7 replies; 9+ messages in thread
From: Abdurrahman Hussain @ 2026-05-16 23:18 UTC (permalink / raw)
To: Guenter Roeck, Alexandru Tachici, Linus Walleij, Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Abdurrahman Hussain
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>
---
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 (5):
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) serialize GPIO PMBus accesses with pmbus_lock
drivers/hwmon/pmbus/adm1266.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
---
base-commit: 70eda68668d1476b459b64e69b8f36659fa9dfa8
change-id: 20260516-adm1266-gpio-fixes-dbdb9c10a4c2
Best regards,
--
Abdurrahman Hussain <abdurrahman@nexthop.ai>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/5] hwmon: (pmbus/adm1266) cap PDIO scan in get_multiple at ADM1266_PDIO_NR
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
@ 2026-05-16 23:18 ` Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 2/5] hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple Abdurrahman Hussain
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Abdurrahman Hussain @ 2026-05-16 23:18 UTC (permalink / raw)
To: Guenter Roeck, Alexandru Tachici, Linus Walleij, Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Abdurrahman Hussain
adm1266_gpio_get_multiple() iterates the PDIO portion of the
caller-supplied mask using
for_each_set_bit_from(gpio_nr, mask,
ADM1266_GPIO_NR + ADM1266_PDIO_STATUS) {
...
}
where ADM1266_PDIO_STATUS is the PMBus command code (0xE9, i.e. 233),
not the number of PDIO pins. The intended upper bound is
ADM1266_GPIO_NR + ADM1266_PDIO_NR = 25.
gpiolib hands in a mask sized for gc.ngpio (= 25 bits on this chip),
so the iteration walks find_next_bit() up to 242, reading up to 27
extra unsigned-long words of whatever lives past the end of the mask
in the caller's stack. Any incidental set bit in that range then
drives a set_bit(gpio_nr, bits) call that writes past the end of the
caller-supplied bits array too -- both out-of-bounds.
Substitute ADM1266_PDIO_NR for the constant so the scan stops at the
last real PDIO bit.
Fixes: d98dfad35c38 ("hwmon: (pmbus/adm1266) Add support for GPIOs")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/hwmon/pmbus/adm1266.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index d90f8f80be8e..11f9a44f4361 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -211,7 +211,7 @@ static int adm1266_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask
status = read_buf[0] + (read_buf[1] << 8);
*bits = 0;
- for_each_set_bit_from(gpio_nr, mask, ADM1266_GPIO_NR + ADM1266_PDIO_STATUS) {
+ for_each_set_bit_from(gpio_nr, mask, ADM1266_GPIO_NR + ADM1266_PDIO_NR) {
if (test_bit(gpio_nr - ADM1266_GPIO_NR, &status))
set_bit(gpio_nr, bits);
}
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/5] hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 1/5] hwmon: (pmbus/adm1266) cap PDIO scan in get_multiple at ADM1266_PDIO_NR Abdurrahman Hussain
@ 2026-05-16 23:18 ` Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 3/5] hwmon: (pmbus/adm1266) reject short block-read responses in the GPIO accessors Abdurrahman Hussain
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Abdurrahman Hussain @ 2026-05-16 23:18 UTC (permalink / raw)
To: Guenter Roeck, Alexandru Tachici, Linus Walleij, Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Abdurrahman Hussain
adm1266_gpio_get_multiple() zeroes *bits before the GPIO_STATUS loop
and then a second time before the PDIO_STATUS loop:
*bits = 0;
for_each_set_bit(gpio_nr, mask, ADM1266_GPIO_NR) {
...
set_bit(gpio_nr, bits);
}
ret = i2c_smbus_read_block_data(data->client, ADM1266_PDIO_STATUS, ...);
...
*bits = 0;
for_each_set_bit_from(gpio_nr, mask, ADM1266_GPIO_NR + ADM1266_PDIO_NR) {
...
set_bit(gpio_nr, bits);
}
The second *bits = 0 throws away every GPIO bit the first loop just
populated, so callers asking for any combination of GPIO and PDIO
pins always see the GPIO portion of the returned bits as zero.
Drop the redundant second assignment so both halves of the result
survive.
Fixes: d98dfad35c38 ("hwmon: (pmbus/adm1266) Add support for GPIOs")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/hwmon/pmbus/adm1266.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 11f9a44f4361..4dd67c02b412 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -210,7 +210,6 @@ static int adm1266_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask
status = read_buf[0] + (read_buf[1] << 8);
- *bits = 0;
for_each_set_bit_from(gpio_nr, mask, ADM1266_GPIO_NR + ADM1266_PDIO_NR) {
if (test_bit(gpio_nr - ADM1266_GPIO_NR, &status))
set_bit(gpio_nr, bits);
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/5] hwmon: (pmbus/adm1266) reject short block-read responses in the GPIO accessors
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 1/5] hwmon: (pmbus/adm1266) cap PDIO scan in get_multiple at ADM1266_PDIO_NR Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 2/5] hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple Abdurrahman Hussain
@ 2026-05-16 23:18 ` Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 4/5] hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe() Abdurrahman Hussain
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Abdurrahman Hussain @ 2026-05-16 23:18 UTC (permalink / raw)
To: Guenter Roeck, Alexandru Tachici, Linus Walleij, Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Abdurrahman Hussain
adm1266_gpio_get() and adm1266_gpio_get_multiple() both compose the
pin-status word as
pins_status = read_buf[0] + (read_buf[1] << 8);
right after i2c_smbus_read_block_data(), guarding only against an
error return. A well-behaved device returns 2 bytes for
GPIO_STATUS/PDIO_STATUS, but the helper happily reports a 0- or
1-byte response too. If the device returns 0 bytes, both read_buf
slots are uninitialized stack memory; if it returns 1 byte, read_buf[1]
is.
The composed value then flows through set_bit() into the caller's
*bits in adm1266_gpio_get_multiple(), or into the return value of
adm1266_gpio_get(), and ends up in userspace via gpiolib (sysfs and
the char-dev ioctls). That leaks a few bits of kernel stack per
request on any device whose firmware glitch, bus error, or hostile
slave produces a short block-read response.
Add the missing length check to both call sites and surface a short
response as -EIO.
Fixes: d98dfad35c38 ("hwmon: (pmbus/adm1266) Add support for GPIOs")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/hwmon/pmbus/adm1266.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 4dd67c02b412..57cb7d302cdd 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -175,6 +175,8 @@ static int adm1266_gpio_get(struct gpio_chip *chip, unsigned int offset)
ret = i2c_smbus_read_block_data(data->client, pmbus_cmd, read_buf);
if (ret < 0)
return ret;
+ if (ret < 2)
+ return -EIO;
pins_status = read_buf[0] + (read_buf[1] << 8);
if (offset < ADM1266_GPIO_NR)
@@ -195,6 +197,8 @@ static int adm1266_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask
ret = i2c_smbus_read_block_data(data->client, ADM1266_GPIO_STATUS, read_buf);
if (ret < 0)
return ret;
+ if (ret < 2)
+ return -EIO;
status = read_buf[0] + (read_buf[1] << 8);
@@ -207,6 +211,8 @@ static int adm1266_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask
ret = i2c_smbus_read_block_data(data->client, ADM1266_PDIO_STATUS, read_buf);
if (ret < 0)
return ret;
+ if (ret < 2)
+ return -EIO;
status = read_buf[0] + (read_buf[1] << 8);
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 4/5] hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe()
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
` (2 preceding siblings ...)
2026-05-16 23:18 ` [PATCH v2 3/5] hwmon: (pmbus/adm1266) reject short block-read responses in the GPIO accessors Abdurrahman Hussain
@ 2026-05-16 23:18 ` Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 5/5] hwmon: (pmbus/adm1266) serialize GPIO PMBus accesses with pmbus_lock Abdurrahman Hussain
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Abdurrahman Hussain @ 2026-05-16 23:18 UTC (permalink / raw)
To: Guenter Roeck, Alexandru Tachici, Linus Walleij, Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Abdurrahman Hussain
adm1266_probe() calls adm1266_config_gpio() -- which goes on to
devm_gpiochip_add_data() and exposes the gpio_chip callbacks to
gpiolib -- before pmbus_do_probe() has initialised the per-client
PMBus state (notably the pmbus_lock mutex the core hands out via
pmbus_get_data()).
That ordering is already a latent hazard: any GPIO access that lands
between adm1266_config_gpio() and the end of pmbus_do_probe() (for
example a sysfs read from a user space agent that opens the gpiochip
the instant gpiolib advertises it) races pmbus_do_probe()'s own
device accesses with no serialisation.
Move adm1266_config_gpio() down past pmbus_do_probe() so the chip
isn't reachable from userspace until the PMBus state it depends on
is fully initialised.
Fixes: d98dfad35c38 ("hwmon: (pmbus/adm1266) Add support for GPIOs")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/hwmon/pmbus/adm1266.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 57cb7d302cdd..b91dcf067fa6 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -467,10 +467,6 @@ static int adm1266_probe(struct i2c_client *client)
crc8_populate_msb(pmbus_crc_table, 0x7);
mutex_init(&data->buf_mutex);
- ret = adm1266_config_gpio(data);
- if (ret < 0)
- return ret;
-
ret = adm1266_set_rtc(data);
if (ret < 0)
return ret;
@@ -483,6 +479,10 @@ static int adm1266_probe(struct i2c_client *client)
if (ret)
return ret;
+ ret = adm1266_config_gpio(data);
+ if (ret < 0)
+ return ret;
+
adm1266_init_debugfs(data);
return 0;
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 5/5] hwmon: (pmbus/adm1266) serialize GPIO PMBus accesses with pmbus_lock
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
` (3 preceding siblings ...)
2026-05-16 23:18 ` [PATCH v2 4/5] hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe() Abdurrahman Hussain
@ 2026-05-16 23:18 ` Abdurrahman Hussain
2026-05-18 9:08 ` [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Bartosz Golaszewski
2026-05-18 22:08 ` Guenter Roeck
6 siblings, 0 replies; 9+ messages in thread
From: Abdurrahman Hussain @ 2026-05-16 23:18 UTC (permalink / raw)
To: Guenter Roeck, Alexandru Tachici, Linus Walleij, Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Abdurrahman Hussain
adm1266_gpio_get(), adm1266_gpio_get_multiple(), and
adm1266_gpio_dbg_show() all issue PMBus reads against the device but
none of them take pmbus_lock. The pmbus_core framework holds
pmbus_lock around its own multi-transaction sequences (notably the
"set PAGE, then read paged register" pattern used by hwmon
attributes), so an unlocked GPIO accessor can land between a PAGE
write and the subsequent paged read in another thread and corrupt
either side's view of the device state machine.
Take pmbus_lock at the top of each of the three accessors via the
scope-based guard(). The lock is uncontended in the common case and
adds only a single mutex round-trip per call.
Fixes: d98dfad35c38 ("hwmon: (pmbus/adm1266) Add support for GPIOs")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
drivers/hwmon/pmbus/adm1266.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index b91dcf067fa6..2e9ae03ab64b 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -172,6 +172,8 @@ static int adm1266_gpio_get(struct gpio_chip *chip, unsigned int offset)
else
pmbus_cmd = ADM1266_PDIO_STATUS;
+ guard(pmbus_lock)(data->client);
+
ret = i2c_smbus_read_block_data(data->client, pmbus_cmd, read_buf);
if (ret < 0)
return ret;
@@ -194,6 +196,8 @@ static int adm1266_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask
unsigned int gpio_nr;
int ret;
+ guard(pmbus_lock)(data->client);
+
ret = i2c_smbus_read_block_data(data->client, ADM1266_GPIO_STATUS, read_buf);
if (ret < 0)
return ret;
@@ -235,6 +239,8 @@ static void adm1266_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
int ret;
int i;
+ guard(pmbus_lock)(data->client);
+
for (i = 0; i < ADM1266_GPIO_NR; i++) {
write_cmd = adm1266_gpio_mapping[i][1];
ret = adm1266_pmbus_block_xfer(data, ADM1266_GPIO_CONFIG, 1, &write_cmd, read_buf);
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
` (4 preceding siblings ...)
2026-05-16 23:18 ` [PATCH v2 5/5] hwmon: (pmbus/adm1266) serialize GPIO PMBus accesses with pmbus_lock Abdurrahman Hussain
@ 2026-05-18 9:08 ` Bartosz Golaszewski
2026-05-18 22:08 ` Guenter Roeck
6 siblings, 0 replies; 9+ messages in thread
From: Bartosz Golaszewski @ 2026-05-18 9:08 UTC (permalink / raw)
To: Abdurrahman Hussain
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Guenter Roeck,
Alexandru Tachici, Linus Walleij, Bartosz Golaszewski
On Sun, 17 May 2026 01:18:46 +0200, Abdurrahman Hussain
<abdurrahman@nexthop.ai> said:
> 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>
> ---
All these make sense to me.
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
` (5 preceding siblings ...)
2026-05-18 9:08 ` [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Bartosz Golaszewski
@ 2026-05-18 22:08 ` Guenter Roeck
2026-05-19 0:50 ` Abdurrahman Hussain
6 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2026-05-18 22:08 UTC (permalink / raw)
To: Abdurrahman Hussain, Alexandru Tachici, Linus Walleij,
Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes
2026-05-18 22:08 ` Guenter Roeck
@ 2026-05-19 0:50 ` Abdurrahman Hussain
0 siblings, 0 replies; 9+ messages in thread
From: Abdurrahman Hussain @ 2026-05-19 0:50 UTC (permalink / raw)
To: Guenter Roeck, Abdurrahman Hussain, Alexandru Tachici,
Linus Walleij, Bartosz Golaszewski
Cc: linux-hwmon, linux-kernel, stable, linux-gpio, Guenter Roeck
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-19 0:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-16 23:18 [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 1/5] hwmon: (pmbus/adm1266) cap PDIO scan in get_multiple at ADM1266_PDIO_NR Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 2/5] hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 3/5] hwmon: (pmbus/adm1266) reject short block-read responses in the GPIO accessors Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 4/5] hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe() Abdurrahman Hussain
2026-05-16 23:18 ` [PATCH v2 5/5] hwmon: (pmbus/adm1266) serialize GPIO PMBus accesses with pmbus_lock Abdurrahman Hussain
2026-05-18 9:08 ` [PATCH v2 0/5] hwmon: (pmbus/adm1266) GPIO accessor fixes Bartosz Golaszewski
2026-05-18 22:08 ` Guenter Roeck
2026-05-19 0:50 ` Abdurrahman Hussain
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®