Re: [PATCH] Input: adp5588-keys - cache GPIO state before registering the gpiochip

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error handling for I2C reads causes out-of-bounds memory accesses and hardware state corruption.
--

--- Patch [1]: [PATCH] Input: adp5588-keys - cache GPIO state before registering the gpiochip ---
Note: The format of this review has been altered to a free-form summary
due to system recitation restrictions. No direct patch quoting is used.

commit d8a90d44c5e4df663ac4d3f942090902da1d6732
Author: Alvin Šipraga <[email protected]>
Subject: Input: adp5588-keys - cache GPIO state before registering the gpiochip

This commit reorders the GPIO state caching to occur before the
gpiochip is registered. This prevents gpiochip operations from clobbering
pre-programmed GPIO states set by the bootloader.

[Severity: High]
This isn't a bug introduced by this patch, but this patch actively
exacerbates the consequences of a pre-existing issue with missing error
handling on I2C reads. 

If adp5588_read() fails due to a bus fault, it returns a negative error
code (e.g., -EIO). In adp5588_gpio_add(), these negative values are
silently truncated to 8-bit unsigned values and stored in the driver's
cached GPIO state (dat_out, dir, pull_dis). 

Because this patch moves this caching to occur before 
devm_gpiochip_add_data(), when a GPIO hog is subsequently applied, the
driver inadvertently clobbers the real hardware state by writing back this
error-corrupted cache. Can this actively undermine the patch's stated goal
of preserving bootloader GPIO configurations if an I2C read fails during
probe?

Additionally, this same missing error check causes severe issues elsewhere.
In adp5588_report_events(), the error is bitwise-ANDed with KEY_EV_MASK, 
resulting in an invalid, large key value. This yields a calculated code
index that exceeds the bounds of the kpad->keycode array, causing an
out-of-bounds heap read and leaking phantom key presses to userspace. 

Finally, in adp5588_gpio_get_value(), a negative error code evaluates to
true, incorrectly reporting the pin state as HIGH.

Would it be appropriate to add proper error checking to adp5588_read() or
its callers to prevent state corruption and out-of-bounds memory access
during I2C faults?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1