From: Junjie Cao junjie.cao@intel.com
[ Upstream commit 810e154d90f44127239957b06ee51a55553a5815 ]
timbgpio_irq_type() currently accepts offset == ngpio, violating gpiolib's [0..ngpio-1] contract. This can lead to undefined behavior when computing '1 << offset', and it is also inconsistent with users that iterate with for_each_set_bit(..., ngpio).
Tighten the upper bound to reject offset == ngpio. No functional change for in-range offsets.
Signed-off-by: Junjie Cao junjie.cao@intel.com Link: https://lore.kernel.org/r/20250825090850.127163-1-junjie.cao@intel.com Signed-off-by: Bartosz Golaszewski bartosz.golaszewski@linaro.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
**YES**
This commit should be backported to stable kernel trees. Here's my extensive analysis:
## Bug Severity and Impact
The commit fixes a critical **off-by-one boundary check bug** in `timbgpio_irq_type()` at drivers/gpio/gpio-timberdale.c:141. The bug allows `offset == tgpio->gpio.ngpio`, which violates the GPIO subsystem's fundamental contract of valid offsets being in the range `[0..ngpio-1]`.
## Specific Code Analysis
1. **The Bug**: The original check `if (offset < 0 || offset > tgpio->gpio.ngpio)` incorrectly accepts `offset == ngpio` as valid.
2. **Undefined Behavior Risk**: When `offset == ngpio`, the subsequent bit shift operations become problematic: - Line 154: `bflr &= ~(1 << offset)` - Line 157: `lvr |= 1 << offset` - Line 167: `flr |= 1 << offset` - Line 184: `iowrite32(1 << offset, tgpio->membase + TGPIO_ICR)`
Shifting by 32 or more bits (assuming 32-bit registers) results in **undefined behavior** in C, potentially causing: - Memory corruption - System crashes - Security vulnerabilities through out-of-bounds writes
3. **Inconsistency with GPIO Framework**: The bug creates inconsistency with `for_each_set_bit()` usage at line 208, which correctly iterates from 0 to ngpio-1.
## Backport Criteria Met
✓ **Fixes a real bug**: Prevents undefined behavior and potential memory corruption ✓ **Small and contained**: Single-line change (`>` to `>=`) ✓ **No side effects**: Only tightens validation, doesn't change behavior for valid inputs ✓ **No architectural changes**: Simple boundary check fix ✓ **Long-standing bug**: Present since driver introduction in 2009 (commit 35570ac6039ef4) ✓ **Clear fix**: The correction is obvious and mathematically correct ✓ **Low regression risk**: More restrictive validation cannot break correctly functioning code
## Additional Considerations
- The bug has existed for **15+ years** since the driver's introduction, making it a candidate for all maintained stable trees - While the timberdale GPIO driver may not be widely used, the fix prevents potential security issues from invalid array indexing - Similar boundary check issues in kernel drivers have historically been backported to stable - The fix aligns with kernel security best practices of proper input validation
The commit message accurately describes the issue, and the fix is the minimal change necessary to resolve the bug without introducing new functionality or risk.
drivers/gpio/gpio-timberdale.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-timberdale.c b/drivers/gpio/gpio-timberdale.c index cb303a26f4d3c..61f15a864a5ad 100644 --- a/drivers/gpio/gpio-timberdale.c +++ b/drivers/gpio/gpio-timberdale.c @@ -138,7 +138,7 @@ static int timbgpio_irq_type(struct irq_data *d, unsigned trigger) u32 ver; int ret = 0;
- if (offset < 0 || offset > tgpio->gpio.ngpio) + if (offset < 0 || offset >= tgpio->gpio.ngpio) return -EINVAL;
ver = ioread32(tgpio->membase + TGPIO_VER);