All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Abdurrahman Hussain" <abdurrahman@nexthop.ai>
Cc: linux-hwmon@vger.kernel.org
Subject: Re: [PATCH v3 2/8] hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple
Date: Tue, 19 May 2026 01:35:36 +0000	[thread overview]
Message-ID: <20260519013537.229EAC2BCB7@smtp.kernel.org> (raw)
In-Reply-To: <20260518-adm1266-gpio-fixes-v3-2-e425e4f88139@nexthop.ai>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized stack memory use on short I2C block reads
- [High] Missing required locking for PMBus hardware accesses
--

commit feacc189ff4113f817d364f381e4726e168838b1
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple

This commit prevents the pdio loop in adm1266_gpio_get_multiple() from
overwriting previously populated gpio bits by dropping a redundant assignment.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 11f9a44f4361a..4dd67c02b412e 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);

i2c_smbus_read_block_data() returns the number of bytes read on success.

If a short read occurs and fewer than two bytes are returned, does this
result in using uninitialized stack data for read_buf[1]?

I note this regression is fixed later in the series by a commit regarding
short block-read responses in the gpio accessors.

Additionally, the hardware monitoring subsystem guidelines require drivers
to implement locking for attributes registered by other means.

Can these unlocked pmbus reads in adm1266_gpio_get_multiple() interleave
with multi-message transactions from other threads, potentially corrupting
the active page state?

I see this regression is also resolved later in the series by a commit
that serializes gpio pmbus accesses with pmbus_lock.

> -	*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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-adm1266-gpio-fixes-v3-0-e425e4f88139@nexthop.ai?part=2

  reply	other threads:[~2026-05-19  1:35 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19  0:52 [PATCH v3 0/8] hwmon: (pmbus/adm1266) GPIO, NVMEM, and debugfs accessor fixes Abdurrahman Hussain
2026-05-19  0:52 ` [PATCH v3 1/8] hwmon: (pmbus/adm1266) cap PDIO scan in get_multiple at ADM1266_PDIO_NR Abdurrahman Hussain
2026-05-19  1:14   ` sashiko-bot
2026-05-19  0:52 ` [PATCH v3 2/8] hwmon: (pmbus/adm1266) don't clobber GPIO bits before PDIO read in get_multiple Abdurrahman Hussain
2026-05-19  1:35   ` sashiko-bot [this message]
2026-05-19  0:52 ` [PATCH v3 3/8] hwmon: (pmbus/adm1266) reject short block-read responses in the GPIO accessors Abdurrahman Hussain
2026-05-19  1:58   ` sashiko-bot
2026-05-19  0:52 ` [PATCH v3 4/8] hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe() Abdurrahman Hussain
2026-05-19  2:35   ` sashiko-bot
2026-05-19  0:52 ` [PATCH v3 5/8] hwmon: (pmbus/adm1266) register the nvmem device " Abdurrahman Hussain
2026-05-19  3:42   ` sashiko-bot
2026-05-19  0:52 ` [PATCH v3 6/8] hwmon: (pmbus/adm1266) serialize GPIO PMBus accesses with pmbus_lock Abdurrahman Hussain
2026-05-19  4:18   ` sashiko-bot
2026-05-19  0:52 ` [PATCH v3 7/8] hwmon: (pmbus/adm1266) serialize NVMEM blackbox read " Abdurrahman Hussain
2026-05-19  4:54   ` sashiko-bot
2026-05-19  0:52 ` [PATCH v3 8/8] hwmon: (pmbus/adm1266) serialize sequencer_state debugfs " Abdurrahman Hussain
2026-05-20 14:02 ` [PATCH v3 0/8] hwmon: (pmbus/adm1266) GPIO, NVMEM, and debugfs accessor fixes Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260519013537.229EAC2BCB7@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=abdurrahman@nexthop.ai \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.