Linux LED subsystem development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18] leds: core: Fix race condition for software blink
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
@ 2026-08-31 13:23 ` Sasha Levin
  2026-08-31 14:50   ` sashiko-bot
  2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-5.10] leds: pca9532: Don't stop blinking for non-zero brightness Sasha Levin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Sasha Levin @ 2026-08-31 13:23 UTC (permalink / raw)
  To: patches, stable
  Cc: Craig McQueen, Lee Jones, Sasha Levin, pavel, linux-leds,
	linux-kernel

From: Craig McQueen <craig@mcqueen.au>

[ Upstream commit f92135f100669b508dd62b424ab20bcb33494c79 ]

led_set_brightness() function: Change handling of software blink to
avoid race conditions when stopping blink and setting brightness.

Triggers may call led_set_brightness(LED_OFF),
led_set_brightness(LED_FULL) in quick succession to disable blinking and
turn the LED on. If the delayed work task has not yet disabled blinking
by the time the second call occurs, then the brightness also needs to be
changed in the delayed work task.

Signed-off-by: Craig McQueen <craig@mcqueen.au>
Link: https://patch.msgid.link/20260423113638.2079302-1-craig@mcqueen.au
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The background searches finished and line up with the earlier analysis:

- **Commit lookup:** `f92135f100669` (`leds: core: Fix race condition
  for software blink`) is in mainline via the `leds-next-7.2` merge, but
  it is **not** in the current `6.18.44` checkout — so it is a valid
  backport candidate for this tree.
- **Related history:** The prerequisite fixes are already here
  (`fa15d8c69238b`, `2c70953b6f535`, ordered LED workqueue from
  `32360bf6a5d40`).

That supports the **YES** verdict: the bug exists in v6.18.44, the fix
is small and upstream, and the needed infrastructure is already present.

 drivers/leds/led-core.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 59473f286b31f..eeb36e50c285c 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -304,24 +304,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
 
 void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
 {
-	/*
-	 * If software blink is active, delay brightness setting
-	 * until the next timer tick.
-	 */
-	if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
+	if (brightness) {
 		/*
-		 * If we need to disable soft blinking delegate this to the
-		 * work queue task to avoid problems in case we are called
-		 * from hard irq context.
+		 * If software blink disable is pending, also queue brightness setting.
+		 * If software blink is active, delay brightness setting
+		 * until the next timer tick.
 		 */
-		if (!brightness) {
-			set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
+		if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) ||
+		    test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) {
+			led_cdev->delayed_set_value = brightness;
+			set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
 			queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
-		} else {
-			set_bit(LED_BLINK_BRIGHTNESS_CHANGE,
-				&led_cdev->work_flags);
+			return;
+		} else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
 			led_cdev->new_blink_brightness = brightness;
+			set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags);
+			return;
 		}
+	} else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
+		/*
+		 * If we need to disable soft blinking delegate this to the
+		 * work queue task to avoid problems in case we are called
+		 * from hard irq context.
+		 */
+		set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
+		queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
 		return;
 	}
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] leds: pca9532: Don't stop blinking for non-zero brightness
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
  2026-08-31 13:23 ` [PATCH AUTOSEL 6.18] leds: core: Fix race condition for software blink Sasha Levin
@ 2026-08-31 13:23 ` Sasha Levin
  2026-08-31 14:58   ` sashiko-bot
  2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.12] leds: trigger: gpio: Use GPIOD_FLAGS_BIT_NONEXCLUSIVE Sasha Levin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 10+ messages in thread
From: Sasha Levin @ 2026-08-31 13:23 UTC (permalink / raw)
  To: patches, stable
  Cc: Tobias Deiminger, Lee Jones, Sasha Levin, riku.voipio, pavel,
	linux-leds, linux-kernel

From: Tobias Deiminger <tobias.deiminger@linutronix.de>

[ Upstream commit 0261683a4d31783d680e74b3ae5f22f6a62128cc ]

pca9532 unexpectedly stopped blinking when changing brightness to a
non-zero value. To reproduce:

 echo timer > /sys/class/leds/led-1/trigger   # blinks
 echo 255 > /sys/class/leds/led-1/brightness  # blinking stops, light on
 cat /sys/class/leds/led-1/trigger            # still claims [timer]

According to Documentation/leds/leds-class.rst, only brightness = 0
shall be a stop condition:

> You can change the brightness value of a LED independently of the
> timer trigger. However, if you set the brightness value to LED_OFF it
> will also disable the timer trigger.

Therefore add a guard to continue blinking when brightness != LED_OFF,
similar to how pca955x does it since 575f10dc64a2 ("leds: pca955x: Add
HW blink support").

Signed-off-by: Tobias Deiminger <tobias.deiminger@linutronix.de>
Link: https://patch.msgid.link/20260331202848.658676-1-tobias.deiminger@linutronix.de
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `leds: pca9532: Don't stop blinking for non-
zero brightness`

**Local tree:** `v6.18.44` (`6.18.44`, Makefile `VERSION=6`,
`PATCHLEVEL=18`, `SUBLEVEL=44`)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject line
**Record:** `[leds: pca9532]` `[fix/guard]` — When brightness is set to
a non-zero value while hardware blinking is active, do not stop
blinking.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Tobias Deiminger `<tobias.deiminger@linutronix.de>`
  (author)
- **Link:** https://patch.msgid.link/20260331202848.658676-1-
  tobias.deiminger@linutronix.de
- **Signed-off-by:** Lee Jones `<lee@kernel.org>` (subsystem maintainer,
  applied)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
  by:`, or `Cc: stable@vger.kernel.org`
- Lore thread identifies mainline commit as
  `770edd8e8e5bed961af2ca6ab397052046d1d774` (not present in this
  checkout)

### Step 1.3: Body analysis
**Record:**
- **Bug:** After enabling the `timer` trigger (hardware blink via PWM1),
  writing a non-zero value to `brightness` stops blinking while sysfs
  still reports `[timer]`.
- **Symptom:** LED stays solid on; trigger sysfs entry is inconsistent
  with actual behavior.
- **Reproduction:** Documented sysfs sequence (`timer` trigger → `echo
  255 > brightness` → `cat trigger`).
- **Root cause (author):** `pca9532_set_brightness()` overwrites
  `PCA9532_PWM1` state for any non-zero brightness.
- **Reference:** LED class docs say only `LED_OFF` should stop a timer
  trigger; `pca955x` already guards this way since `575f10dc64a2`.

### Step 1.4: Hidden bug fix?
**Record:** No — this is an explicit functional bug fix, not disguised
cleanup.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
**Record:**
- **Files:** `drivers/leds/leds-pca9532.c` only (+4/-2 lines net)
- **Function:** `pca9532_set_brightness()`
- **Scope:** Single-file, surgical driver fix

### Step 2.2: Code flow change
**Record:**
- **Before:** Any non-zero brightness overwrites `led->state`
  (`PCA9532_ON` or `PCA9532_PWM0`), including when already in
  `PCA9532_PWM1` (HW blink).
- **After:** If `value == LED_OFF` → turn off as before. Else if
  `led->state == PCA9532_PWM1` → return 0 immediately (preserve HW
  blink). Otherwise unchanged logic for `LED_FULL` / PWM dimming.
- **Path affected:** Sysfs `brightness` writes while hardware blinking
  is active.

### Step 2.3: Bug mechanism
**Record:**
- **Category:** Logic / correctness fix (API contract violation)
- **Mechanism:** HW blink sets `led->state = PCA9532_PWM1` in
  `pca9532_update_hw_blink()`. Subsequent `brightness_set_blocking`
  calls clobber that state via `pca9532_setled()`, stopping hardware
  blink while the LED core still believes the timer trigger is active.

### Step 2.4: Fix quality
**Record:**
- Fix is minimal and mirrors the established `pca955x` pattern
  (`test_bit(active_blink)` → early `goto out` for non-zero brightness).
- **Regression risk:** Low. `PCA9532_PWM1` for HW blink is only set via
  `pca9532_update_hw_blink()`, which requires `hw_blink == true`. The
  N2100 beeper path uses PWM1 but does not register a `led_classdev`
  brightness callback, so the guard does not affect beeper input
  handling.
- `LED_OFF` still correctly stops the LED.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
**Record:** `pca9532_set_brightness()` core logic dates to 2008
(`e14fa82439d33c`). The bug was latent until HW blink landed in
`48ca7f302cfcf` (2024-06-17, "leds: pca9532: Use PWM1 for hardware
blinking"), which added `pca9532_update_hw_blink()` setting
`PCA9532_PWM1` without updating `pca9532_set_brightness()`.

### Step 3.2: Fixes: tag
**Record:** N/A — no `Fixes:` tag. Logical introducer is
`48ca7f302cfcf`, which is an ancestor of this tree.

### Step 3.3: Related file history
**Record:** Recent `leds-pca9532.c` commits in this tree include HW
blink work (`48ca7f302cfcf`, `f51bc3cedfc45`), default frequency change,
and error-message cleanup (`2aad93b6de0d8`, which carried `Cc: stable`).
This fix is standalone (v2 of a single-patch series).

### Step 3.4: Author context
**Record:** Tobias Deiminger has no other commits in `drivers/leds/` in
this tree. Lee Jones (LED maintainer) applied the patch.

### Step 3.5: Dependencies
**Record:**
- Requires HW blink support (`48ca7f302cfcf`) — **present** in this
  tree.
- References `pca955x` pattern from `575f10dc64a2` — **present** in this
  tree.
- No series dependencies; applies standalone.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original discussion
**Record:**
- **URL:** https://lore.kernel.org/linux-
  leds/20260331202848.658676-1-tobias.deiminger@linutronix.de/
- **Series:** v2 (v1 at https://lore.kernel.org/r/20260321102121.1563365
  -1-tobias.deiminger@linutronix.de); v2 only changes comment style and
  brace placement.
- **Review:** Lee Jones replied "Applied, thanks!" — no NAKs or
  objections found.
- **Stable nomination:** None in thread.

### Step 4.2: Reviewers
**Record:** CC'd: `lee@kernel.org`, `pavel@kernel.org`,
`eajames@linux.ibm.com`, `riku.voipio@iki.fi`, `linux-
leds@vger.kernel.org`. Lee Jones (maintainer) applied.

### Step 4.3: Bug report
**Record:** Author-provided sysfs reproduction in patch and commit
message. No external bugzilla/syzbot report.

### Step 4.4: Related patches
**Record:** Standalone 1/1 patch. Related context: `575f10dc64a2`
(pca955x HW blink guard) and `48ca7f302cfcf` (pca9532 HW blink
introduction).

### Step 4.5: Stable list
**Record:** No stable-list discussion found for this specific fix.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `pca9532_set_brightness()` (modified); context:
`pca9532_update_hw_blink()`, `pca9532_set_blink()`, `pca9532_setled()`.

### Step 5.2: Callers
**Record:** `pca9532_set_brightness` is registered as
`brightness_set_blocking` for `PCA9532_TYPE_LED` devices (probe path
~line 426). Called from LED core via `__led_set_brightness_blocking()`
on sysfs `brightness` writes — userspace-accessible.

### Step 5.3: Callees
**Record:** On guarded path: none (early return). Normal path:
`pca9532_calcpwm()`, `pca9532_setpwm()`, `pca9532_setled()` (I2C
register writes under mutex).

### Step 5.4: Reachability
**Record:**
1. User writes `timer` to `trigger` → `led_blink_set()` →
   `pca9532_set_blink()` → HW blink configures PWM1
2. User writes non-zero `brightness` → `pca9532_set_brightness()` —
   **buggy without fix**
- Reachable from unprivileged userspace (sysfs, subject to permissions).
  Common on embedded status-LED setups.

### Step 5.5: Similar patterns
**Record:** `pca955x_led_set()` in `leds-pca955x.c` lines 316–323
explicitly preserves blinking for non-zero brightness when
`active_blink` is set — same design intent.

---

## PHASE 6: CROSS-REFERENCING AGAINST LOCAL TREE

### Step 6.1: Buggy code present?
**Record:** **Yes.** Current `pca9532_set_brightness()` at lines 185–198
lacks the `PCA9532_PWM1` guard. HW blink support from `48ca7f302cfcf` is
in this tree. Bug introduced ~2024-06 with that commit.

### Step 6.2: Backport complications
**Record:** Expected **clean apply** — the surrounding function matches
the diff context exactly. No conflicting recent changes to this
function.

### Step 6.3: Related fixes already present?
**Record:** **No** — grep finds no "Non-zero brightness shall not stop"
comment or equivalent guard. The fix commit (`770edd8e8e5b`) is not in
this tree.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem criticality
**Record:** `drivers/leds/` — **PERIPHERAL** driver (PCA9532 I2C LED
controller). Important for embedded/industrial boards (e.g. historical
Thecus NAS platforms), not core kernel.

### Step 7.2: Subsystem activity
**Record:** LED subsystem actively maintained in 6.18.y; recent stable-
relevant fixes include buffer overread, error-path leaks, and probe-
order fixes.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who is affected
**Record:** Users of `pca9532` with `hw_blink == true` (default for
normal LED configs; disabled only for N2100 beeper variant). Driver-
specific, config/board-specific.

### Step 8.2: Trigger conditions
**Record:** Enable `timer` trigger (HW blink succeeds), then write any
non-zero brightness. **Common** for scripts/users adjusting LED
intensity while blinking. Unprivileged users can trigger via sysfs (with
normal permissions).

### Step 8.3: Failure mode severity
**Record:** Incorrect LED behavior + inconsistent sysfs state (trigger
shows active, hardware not blinking). **Severity: MEDIUM** — no crash,
corruption, or deadlock; real functional regression against documented
LED class behavior.

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Restores documented sysfs semantics; fixes regression
  introduced by HW blink backport already in this tree.
- **Risk:** Very low (4-line guard, proven sibling-driver pattern).
- **Ratio:** Favorable — low-risk regression fix for functionality
  already shipped in 6.18.y.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Real, reproducible bug with clear sysfs steps
- Regression from `48ca7f302cfcf`, already in this tree
- Violates `Documentation/leds/leds-class.rst` contract
- Tiny, obviously correct fix matching `pca955x`
- Maintainer (Lee Jones) applied without objection
- Clean apply expected

**AGAINST backport:**
- Not a crash, security, corruption, or deadlock issue
- Affects only `pca9532` + HW blink configurations
- No explicit `Cc: stable` or user bug reports beyond author

**Unresolved:** None material to the decision.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — logic matches pca955x; sysfs
   repro documented; maintainer applied.
2. Fixes a real bug affecting users? **PASS** — sysfs/API behavior bug
   on real hardware.
3. Important issue? **PASS (moderate)** — regression in shipped HW-blink
   feature; inconsistent sysfs state; not crash-level but user-visible
   and documented-API violation.
4. Small and contained? **PASS** — ~4 lines, one function, one file.
5. No new features or APIs? **PASS** — behavior correction only.
6. Can apply to local tree? **PASS** — prerequisite commits present;
   clean apply expected.

### Step 9.3: Exception categories
**Record:** None (not device ID, quirk, DT, build, or docs-only).
Standard driver correctness fix.

### Step 9.4: Problem and decision rationale

This commit fixes a regression introduced when hardware blinking was
added to `pca9532` in this tree (`48ca7f302cfcf`). Without the guard,
any non-zero `brightness` write silently tears down `PCA9532_PWM1`
hardware blink while the LED core still reports the `timer` trigger as
active — breaking the documented rule that non-zero brightness changes
should not disable the timer trigger.

For 6.18.y users who received HW blink support, this completes that
feature correctly. The fix is minimal, follows an in-tree precedent
(`pca955x`), and carries negligible regression risk. While not a crash
fix, it is a clear regression fix for behavior already present in this
stable tree.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body from user-provided commit
  message; fetched lore v2 mbox via curl
- **[Phase 2]** Read current `pca9532_set_brightness()` at lines
  179–199; analyzed provided diff
- **[Phase 3]** `git describe HEAD` → v6.18.44; `git blame -L 179,199` →
  core logic from 2008, bug latent until 2024; `git log --oneline -20 --
  drivers/leds/leds-pca9532.c` → `48ca7f302cfcf` present; `git merge-
  base --is-ancestor` → both `48ca7f302cfcf` and `575f10dc64a2` in tree;
  `git show 48ca7f302cfcf` confirmed HW blink addition
- **[Phase 4]** curl lore mbox: v2 patch, Lee Jones "Applied, thanks!",
  commit id `770edd8e8e5b`; `b4 dig -c` failed (commit not in tree); no
  stable-list hits
- **[Phase 5]** Traced `brightness_set_blocking` registration (~line
  426); read `pca955x_led_set()` guard at lines 316–323; read
  `led_blink_setup()` in `led-core.c` (HW blink bypasses SW timer when
  `blink_set` succeeds)
- **[Phase 6]** Confirmed buggy code present (no PCA9532_PWM1 guard);
  grep found no existing fix; `git show 770edd8e8e5b` → bad object (not
  in tree)
- **[Phase 7]** `git log --oneline -20 -- drivers/leds/` for subsystem
  activity
- **[Phase 8]** Read `Documentation/leds/leds-class.rst` lines 23–25;
  confirmed `hw_blink` default true except N2100 beeper (~line 440)
- **[Phase 9]** Synthesized against stable rules for 6.18.44 only

**YES**The background search finished: the fix (`leds: pca9532: Don't
stop blinking for non-zero brightness`) is **not** in this `6.18.44`
tree yet. The buggy code is still there — `pca9532_set_brightness()` can
overwrite `PCA9532_PWM1` when sysfs brightness is set to a non-zero
value while hardware blinking is active.

**Verdict for this tree: YES** — small regression fix for HW blink
support already in 6.18.y; low risk and matches the `pca955x` pattern.

 drivers/leds/leds-pca9532.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index 0344189bb991c..dae7c67605084 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -182,11 +182,13 @@ static int pca9532_set_brightness(struct led_classdev *led_cdev,
 	int err = 0;
 	struct pca9532_led *led = ldev_to_led(led_cdev);
 
-	if (value == LED_OFF)
+	if (value == LED_OFF) {
 		led->state = PCA9532_OFF;
-	else if (value == LED_FULL)
+	} else if (led->state == PCA9532_PWM1) {
+		return 0; /* Non-zero brightness shall not stop HW blinking */
+	} else if (value == LED_FULL) {
 		led->state = PCA9532_ON;
-	else {
+	} else {
 		led->state = PCA9532_PWM0; /* Thecus: hardcode one pwm */
 		err = pca9532_calcpwm(led->client, PCA9532_PWM_ID_0, 0, value);
 		if (err)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH AUTOSEL 6.18-6.12] leds: trigger: gpio: Use GPIOD_FLAGS_BIT_NONEXCLUSIVE
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
  2026-08-31 13:23 ` [PATCH AUTOSEL 6.18] leds: core: Fix race condition for software blink Sasha Levin
  2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-5.10] leds: pca9532: Don't stop blinking for non-zero brightness Sasha Levin
@ 2026-08-31 13:26 ` Sasha Levin
  2026-08-31 15:49   ` sashiko-bot
  2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] leds: uleds: Return -EFAULT on copy_to_user() failure Sasha Levin
  2026-08-31 13:29 ` [PATCH AUTOSEL 6.18] leds: tps6131x: Increase overvoltage protection threshold to 6V Sasha Levin
  4 siblings, 1 reply; 10+ messages in thread
From: Sasha Levin @ 2026-08-31 13:26 UTC (permalink / raw)
  To: patches, stable
  Cc: Piotr Kubik, Lee Jones, Sasha Levin, pavel, linux-leds,
	linux-kernel

From: Piotr Kubik <piotr@kubik.pl>

[ Upstream commit 26e15f2558f66b6747ef981df8054d0d92d01b60 ]

When a GPIO is shared between the LED trigger driver and another driver,
the LED trigger driver needs to request the GPIO with
GPIOD_FLAGS_BIT_NONEXCLUSIVE to allow both drivers to monitor the same
GPIO pin.

Without this flag, if another driver has already claimed the GPIO,
the LED trigger driver's gpiod_get_optional() call fails silently,
and the LED trigger doesn't work.

This is needed for scenarios like:
- SFP module presence/status LED triggered by SFP Mod_ABS/Rx_LOS

Both GPIOs are also monitored by the SFP driver for module state
management, so they need to be shared.

Signed-off-by: Piotr Kubik <piotr@kubik.pl>
Link: https://patch.msgid.link/20260408115106.379834-1-piotr@kubik.pl
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
**Record:** `[leds: trigger: gpio]` `[Use]` — Add
`GPIOD_FLAGS_BIT_NONEXCLUSIVE` when requesting the GPIO used by the gpio
LED trigger.

### Step 1.2: Tags
**Record:**
- **Link:**
  https://patch.msgid.link/20260408115106.379834-1-piotr@kubik.pl
- **Signed-off-by:** Piotr Kubik \<piotr@kubik.pl\>
- **Signed-off-by:** Lee Jones \<lee@kernel.org\> (LED subsystem
  maintainer)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, or Cc:
  stable tags
- Notable: maintainer (Lee Jones) Signed-off-by is a quality signal

### Step 1.3: Body Analysis
**Record:**
- **Bug:** When a GPIO is shared between the gpio LED trigger and
  another driver (e.g. SFP `Mod_ABS` / `Rx_LOS`), the LED trigger
  requests the GPIO exclusively; if another driver already claimed it,
  acquisition fails and the trigger does not work.
- **Symptom:** Status LEDs driven by the gpio trigger do not function on
  shared-GPIO hardware.
- **Use case:** SFP module presence/status LEDs on network appliances
  where the SFP driver also monitors the same GPIO lines.
- **Root cause:** Missing `GPIOD_FLAGS_BIT_NONEXCLUSIVE` on
  `gpiod_get_optional()`.

### Step 1.4: Hidden Bug Fix?
**Record:** Yes. Although not labeled "fix", this is a functional bug
fix: shared-GPIO hardware enablement, not a refactor or optimization.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **File:** `drivers/leds/trigger/ledtrig-gpio.c` (+2 / -1 lines)
- **Function:** `gpio_trig_activate()`
- **Scope:** Single-file, surgical one-line functional change

### Step 2.2: Code Flow Change
**Record:**
- **Before:** `gpiod_get_optional(dev, "trigger-sources", GPIOD_IN)` —
  exclusive GPIO request.
- **After:** Same call with `GPIOD_IN | GPIOD_FLAGS_BIT_NONEXCLUSIVE` —
  allows sharing with an already-claimed GPIO.
- **Path:** LED trigger activation during default-trigger setup or
  manual trigger selection.

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Logic / hardware-workaround (shared GPIO access).
- **Mechanism:** Without `NONEXCLUSIVE`, `gpiod_request()` returns
  `-EBUSY` when another consumer already holds the line.
  `gpiod_get_optional()` propagates `ERR_PTR(-EBUSY)`.
  `gpio_trig_activate()` returns that error.
  `led_match_default_trigger()` ignores the `led_trigger_set()` return
  value, so activation failure is silent and the LED never gets the gpio
  trigger.

Verified in gpiolib:

```4672:4674:drivers/gpio/gpiolib.c
        if (ret) {
                if (!(ret == -EBUSY && flags &
GPIOD_FLAGS_BIT_NONEXCLUSIVE))
                        return ERR_PTR(ret);
```

And in LED core:

```271:278:drivers/leds/led-triggers.c
static bool led_match_default_trigger(struct led_classdev *led_cdev,
                                      struct led_trigger *trig)
{
        if (!strcmp(led_cdev->default_trigger, trig->name) &&
            trigger_relevant(led_cdev, trig)) {
                led_cdev->flags |= LED_INIT_DEFAULT_TRIGGER;
                led_trigger_set(led_cdev, trig);
                return true;
```

### Step 2.4: Fix Quality
**Record:**
- Obviously correct: matches the established pattern used across many
  drivers in this tree (regulators, extcon, PHY drivers, etc.).
- Minimal change, no API changes.
- Low regression risk: only affects the shared-GPIO case; exclusive
  GPIOs behave as before.
- The flag is marked deprecated in `consumer.h`, but remains the
  supported workaround throughout this tree.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:**
- Buggy line introduced by `9bbd6b7209cf1` (Andy Shevchenko, Nov 3
  2023): switched to `gpiod_get_optional()` without `NONEXCLUSIVE`.
- Underlying trigger-sources design from `4a11dbf04f31c` (Linus Walleij,
  Sep 26 2023).
- Both commits are present in this 6.18.44 tree.

### Step 3.2: Fixes: Tag
**Record:** N/A — no Fixes: tag in commit message.

### Step 3.3: Related File History
**Record:** Recent `ledtrig-gpio.c` history is cleanups only (sysfs,
kstrtox, DEVICE_ATTR_RW). No related fix already present. Standalone
patch, not part of a series.

### Step 3.4: Author Context
**Record:** Piotr Kubik has other networking-related work in this tree
(e.g. PSE driver). This gpio-trigger fix is a focused hardware-
enablement change, not a large series.

### Step 3.5: Dependencies
**Record:**
- Requires `GPIOD_FLAGS_BIT_NONEXCLUSIVE` (present since
  `ec757001c818c`, 2018).
- Requires trigger-sources gpio trigger (`4a11dbf04f31c`, 2023) —
  present in this tree.
- No other commits required. Applies standalone.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:** `b4 dig -c <commit>` could not run — commit is not in this
checkout. Lore/patch.msgid.link fetch blocked by Anubis bot protection.
Could not retrieve thread discussion.

### Step 4.2: Reviewers
**Record:** UNVERIFIED — could not run `b4 dig -w` without commit hash.
Lee Jones Signed-off-by indicates maintainer acceptance.

### Step 4.3: Bug Report
**Record:** No external bug report or syzbot link. Use case described in
commit message (SFP Mod_ABS/Rx_LOS LEDs).

### Step 4.4: Related Patches
**Record:** No multi-patch series indicated. SFP driver
(`drivers/net/phy/sfp.c`) claims GPIOs via `devm_gpiod_get_optional()`
without `NONEXCLUSIVE` at lines 3149–3150 — consistent with SFP-probes-
first, LED-joins-shared scenario described in the commit.

### Step 4.5: Stable List History
**Record:** UNVERIFIED — could not search lore stable list due to fetch
restrictions.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `gpio_trig_activate()` modified.

### Step 5.2: Callers
**Record:** `gpio_trig_activate` is the `.activate` callback for
`gpio_led_trigger`, invoked from `led_trigger_set()` in
`drivers/leds/led-triggers.c` during:
- Default trigger setup at LED registration (`led_trigger_set_default()`
  → `led_match_default_trigger()`)
- Late trigger module load (`led_trigger_register()`)
- Manual trigger changes via sysfs

### Step 5.3: Callees
**Record:** `gpiod_get_optional()`, `gpiod_set_consumer_name()`,
`request_threaded_irq()` (already uses `IRQF_SHARED`),
`gpio_trig_irq()`.

### Step 5.4: Reachability
**Record:** Triggered during device probe/LED registration for any
platform using `linux,default-trigger = "gpio"` with `trigger-sources`
referencing a GPIO also claimed by another driver. Requires
`CONFIG_LEDS_TRIGGER_GPIO`. SFP network appliances are the documented
case.

### Step 5.5: Similar Patterns
**Record:** `GPIOD_FLAGS_BIT_NONEXCLUSIVE` is used in 30+ locations in
this tree for the same shared-GPIO pattern (regulators, extcon, micrel
PHY, etc.). LED gpio trigger was an omission.

---

## Phase 6: Cross-Reference Against Local Tree (6.18.44)

### Step 6.1: Buggy Code Present?
**Record:** YES. Current tree at `drivers/leds/trigger/ledtrig-
gpio.c:89`:

```89:89:drivers/leds/trigger/ledtrig-gpio.c
        gpio_data->gpiod = gpiod_get_optional(dev, "trigger-sources",
GPIOD_IN);
```

Fix is not yet applied. Bug has existed since Nov 2023 in this tree.

### Step 6.2: Backport Complications
**Record:** Clean apply expected — single-line change, no surrounding
churn. No conflicts anticipated.

### Step 6.3: Related Fixes Already Present?
**Record:** No — `git log --grep` found no matching fix in this tree.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem
**Record:** `drivers/leds/trigger/` — LED triggers. **Criticality:
PERIPHERAL** (status LEDs), but on network appliances SFP status LEDs
are operationally important.

### Step 7.2: Activity
**Record:** Moderate recent activity (cleanups in 2024–2025). trigger-
sources gpio trigger is mature in this tree.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Config-specific — systems with `CONFIG_LEDS_TRIGGER_GPIO`
and device trees wiring gpio LED triggers to GPIO lines shared with
another driver (documented: SFP Mod_ABS/Rx_LOS). Not universal, but real
on network appliance hardware.

### Step 8.2: Trigger Conditions
**Record:**
- Another driver (e.g. SFP) claims the GPIO before LED gpio trigger
  activates.
- Common probe order on SFP platforms (SFP probes first).
- Not a security issue; unprivileged users cannot trigger this directly.

### Step 8.3: Failure Mode Severity
**Record:** Status LEDs silently non-functional. **Severity: LOW to
MEDIUM** — no crash, corruption, or deadlock, but broken hardware
indication on affected platforms.

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Restores intended SFP/status LED behavior on shared-GPIO
  platforms; one-line fix following established kernel pattern.
- **Risk:** Very low — minimal, well-understood change.
- **Ratio:** Moderate benefit, very low risk. Fits hardware-
  quirk/workaround exception category.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real, verified bug with understood mechanism (EBUSY on shared GPIO →
  silent activation failure).
- Small, obviously correct, maintainer-reviewed fix.
- Buggy code and all prerequisites exist in 6.18.44.
- Established `GPIOD_FLAGS_BIT_NONEXCLUSIVE` pattern used widely in this
  tree.
- Hardware workaround for network appliance SFP LED use case.

**AGAINST backport:**
- Not a crash, security, corruption, or deadlock issue.
- Affects a narrow configuration (shared GPIO + gpio LED trigger).
- `GPIOD_FLAGS_BIT_NONEXCLUSIVE` is deprecated (though still the
  required workaround).
- No in-tree DTS found yet wiring SFP GPIOs to gpio LED triggers (use
  case may be out-of-tree/future DTS).

**UNRESOLVED:**
- Full mailing list review thread (fetch blocked).
- Whether any in-tree DTS currently triggers this exact SFP+gpio-trigger
  configuration.

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — mechanism verified in code;
   maintainer SOB; pattern proven across tree.
2. Fixes a real bug affecting users? **PASS** — shared-GPIO LED trigger
   silently fails.
3. Important issue? **PASS (borderline)** — not crash/security, but real
   hardware functionality loss on network appliances; fits hardware-
   workaround exception.
4. Small and contained? **PASS** — 1-line functional change.
5. No new features or APIs? **PASS** — fixes existing trigger behavior.
6. Can apply to local tree? **PASS** — prerequisites present, clean
   apply expected.

### Step 9.3: Exception Category
**Record:** Hardware quirk/workaround — shared GPIO access between LED
trigger and another driver (SFP).

### Step 9.4: Decision Rationale
For the **6.18.44** tree checked out here: the bug exists, the fix is
minimal and follows a well-established kernel pattern, and it restores
broken status-LED functionality on real shared-GPIO hardware (SFP
network appliances). Severity is below crash/corruption tier, but stable
trees routinely accept such hardware-enablement GPIO sharing fixes.
Benefit outweighs the very low regression risk.

---

## Verification

- [Phase 1] Parsed subject, tags, body; identified hardware-sharing bug
- [Phase 2] Read diff and `ledtrig-gpio.c`; verified EBUSY → silent
  failure path via `gpiolib.c:4672-4674` and `led-triggers.c:271-278`
- [Phase 3] `git blame`: buggy line from `9bbd6b7209cf1` (Nov 2023);
  trigger-sources from `4a11dbf04f31c` (Sep 2023)
- [Phase 3] `git log --oneline -20 -- drivers/leds/trigger/ledtrig-
  gpio.c`: no duplicate fix
- [Phase 3] `git show ec757001c818c`: NONEXCLUSIVE support since 2018
- [Phase 4] `b4 dig`: failed — commit not in tree
- [Phase 4] WebFetch lore/patch.msgid.link: blocked by Anubis
- [Phase 5] Traced `gpio_trig_activate` → `led_trigger_set` →
  `led_match_default_trigger` call chain
- [Phase 5] `grep GPIOD_FLAGS_BIT_NONEXCLUSIVE`: 30+ existing uses in
  tree
- [Phase 5] Read `sfp.c:3149-3150`: SFP claims GPIOs without
  NONEXCLUSIVE
- [Phase 6] `git describe HEAD`: v6.18.44-1-g2736c32da98b9
- [Phase 6] Confirmed buggy code at `ledtrig-gpio.c:89` without fix
- [Phase 6] `grep trigger-sources *.dts*`: USB router use cases present;
  no in-tree SFP+gpio-trigger DTS found
- [Phase 8] Confirmed `gpiod_not_found()` only maps `-ENOENT` to NULL,
  not `-EBUSY` (`gpiolib.h:221`)

**YES**The background search finished. It turned up related work
(Puzzle-M801 board DTS with SFP+, Broadcom PHY/SFP LED handling), but
nothing that changes the backport call.

**Verdict for this 6.18.44 tree: YES**

The gpio LED trigger fails silently when another driver (e.g. SFP for
Mod_ABS/Rx_LOS) already owns the GPIO. Adding
`GPIOD_FLAGS_BIT_NONEXCLUSIVE` is a one-line fix that matches an
established pattern elsewhere in the tree, and the buggy code is still
present at `ledtrig-gpio.c:89`.

 drivers/leds/trigger/ledtrig-gpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
index 7f6a2352b0acd..3199275fd250b 100644
--- a/drivers/leds/trigger/ledtrig-gpio.c
+++ b/drivers/leds/trigger/ledtrig-gpio.c
@@ -86,7 +86,8 @@ static int gpio_trig_activate(struct led_classdev *led)
 	 * The generic property "trigger-sources" is followed,
 	 * and we hope that this is a GPIO.
 	 */
-	gpio_data->gpiod = gpiod_get_optional(dev, "trigger-sources", GPIOD_IN);
+	gpio_data->gpiod = gpiod_get_optional(dev, "trigger-sources",
+					      GPIOD_IN | GPIOD_FLAGS_BIT_NONEXCLUSIVE);
 	if (IS_ERR(gpio_data->gpiod)) {
 		ret = PTR_ERR(gpio_data->gpiod);
 		kfree(gpio_data);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] leds: uleds: Return -EFAULT on copy_to_user() failure
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.12] leds: trigger: gpio: Use GPIOD_FLAGS_BIT_NONEXCLUSIVE Sasha Levin
@ 2026-08-31 13:29 ` Sasha Levin
  2026-08-31 17:13   ` sashiko-bot
  2026-08-31 13:29 ` [PATCH AUTOSEL 6.18] leds: tps6131x: Increase overvoltage protection threshold to 6V Sasha Levin
  4 siblings, 1 reply; 10+ messages in thread
From: Sasha Levin @ 2026-08-31 13:29 UTC (permalink / raw)
  To: patches, stable
  Cc: Yousef Alhouseen, Lee Jones, Sasha Levin, pavel, linux-leds,
	linux-kernel

From: Yousef Alhouseen <alhouseenyousef@gmail.com>

[ Upstream commit 61ed78f55a46e12afd4b464c4ba736f55ff33c5e ]

uleds_read() copies the current brightness value to userspace but
ignores copy_to_user() failures. It then clears the pending update and
reports a successful full read even when no data was copied.

Return -EFAULT when the copy fails and leave the update pending so a
later read can retry.

Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
Link: https://patch.msgid.link/20260521181205.15130-1-alhouseenyousef@gmail.com
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
**Record:** `[leds: uleds]` `[Return]` — Return `-EFAULT` on
`copy_to_user()` failure in `uleds_read()`.

### Step 1.2: Tags
**Record:**
- **Link:** `https://patch.msgid.link/20260521181205.15130-1-
  alhouseenyousef@gmail.com`
- **Signed-off-by:** Yousef Alhouseen `<alhouseenyousef@gmail.com>`
  (author)
- **Signed-off-by:** Lee Jones `<lee@kernel.org>` (LED subsystem
  maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
  by:`, or `Cc: stable@vger.kernel.org`
- Notable: maintainer (Lee Jones) sign-off; no syzbot/user reports

### Step 1.3: Body Analysis
**Record:**
- **Bug:** `uleds_read()` calls `copy_to_user()` but ignores its return
  value, then unconditionally clears `new_data` and returns
  `sizeof(udev->brightness)` as success.
- **Symptom:** On `copy_to_user()` failure, userspace gets a successful
  read (positive return) with no data copied; the pending brightness
  update is discarded.
- **Root cause:** Return value overwritten; state cleared regardless of
  copy outcome.
- **Fix:** Return `-EFAULT` on failure; leave `new_data` set so a later
  read can retry.
- **Version info:** None in message.

### Step 1.4: Hidden Bug Fix?
**Record:** No — this is an explicit error-handling bug fix, not
disguised cleanup.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **Files:** `drivers/leds/uleds.c` only (+6 / -3 net)
- **Function:** `uleds_read()`
- **Scope:** Single-file surgical fix in one function

### Step 2.2: Code Flow Change
**Record:**
- **Hunk (lines 150–155):**
  - **Before:** `copy_to_user()` → always `new_data = false` → always
    `retval = sizeof(brightness)` (success).
  - **After:** On `copy_to_user()` failure → `retval = -EFAULT`,
    `new_data` stays true. On success → clear `new_data`, return byte
    count.
- **Path affected:** Read path when `udev->new_data` is true (brightness
  update delivery to userspace).

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Logic / correctness — ignored error return + incorrect
  state transition.
- **Mechanism:** `copy_to_user()` returns bytes-not-copied (0 =
  success). Old code stored this in `retval` then overwrote it. Failed
  copies still cleared `new_data`, losing the event.

### Step 2.4: Fix Quality
**Record:**
- **Quality:** Obviously correct; matches `uleds_write()`
  (`copy_from_user` → `-EFAULT`) and `uinput.c` patterns.
- **Risk:** Very low — only changes the error path; success path
  unchanged.
- **Red flags:** None.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Buggy lines introduced in `e381322b0190c` ("leds: Introduce
userspace LED class driver", Sep 2016). Present unchanged in this tree.

### Step 3.2: Fixes: Tag
**Record:** N/A — no `Fixes:` tag.

### Step 3.3: File History
**Record:** Recent `uleds.c` changes in this tree:
- `6dd51d84a9502` — buffer overread fix (stable backport, `Cc: stable`)
- `cb787f4ac0c2e` — `stream_open` conversion
- `a916d720ab5b4` — `module_misc_device` macro
- Original `e381322b0190c` — driver introduction

Standalone fix; not part of a series.

### Step 3.4: Author Context
**Record:** Yousef Alhouseen has no other commits in `drivers/leds/` in
this tree. Lee Jones committed the related stable backport
`6dd51d84a9502`.

### Step 3.5: Dependencies
**Record:** None. Applies directly to existing `uleds_read()` code.
Standalone.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:** `b4 dig -c 470015e3f8020` failed (commit not in tree).
Lore/patch.msgid.link blocked by bot protection. **UNVERIFIED:** full
review thread, stable nominations, NAKs.

### Step 4.2: Reviewers
**Record:** **UNVERIFIED** (`b4 dig -w` unavailable without commit
hash).

### Step 4.3: Bug Report
**Record:** No `Reported-by:` or bugzilla/syzbot links. Code-review
finding, not a user crash report.

### Step 4.4: Related Patches
**Record:** Related stable-worthy fix in same file: `6dd51d84a9502`
(buffer overread). Independent issue.

### Step 4.5: Stable List History
**Record:** **UNVERIFIED** — lore stable search inaccessible.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `uleds_read()` modified.

### Step 5.2: Callers
**Record:** `uleds_read` is the `.read` handler in `uleds_fops` (line
201). Invoked via `read()` syscall on `/dev/uleds` by userspace (e.g.
`tools/leds/uledmon.c`).

### Step 5.3: Callees
**Record:** `mutex_lock_interruptible`, `copy_to_user`, `mutex_unlock`,
`wait_event_interruptible`.

### Step 5.4: Reachability
**Record:** Userspace opens `/dev/uleds`, writes device registration,
then reads brightness updates. Reachable from unprivileged userspace if
device node permissions allow (standard misc device). `copy_to_user()`
fails on invalid/unmapped userspace buffers.

### Step 5.5: Similar Patterns
**Record:** `uleds_write()` correctly returns `-EFAULT` on
`copy_from_user()` failure (lines 97–100). `uinput.c` consistently
returns `-EFAULT` on `copy_to_user()` failure. `uleds_read()` is the
outlier.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** **YES.** Local tree is **6.18.44** (`git describe HEAD` →
`v6.18.44-1-g2736c32da98b9`). Buggy code at lines 151–154:

```151:154:drivers/leds/uleds.c
                        retval = copy_to_user(buffer, &udev->brightness,
                                              sizeof(udev->brightness));
                        udev->new_data = false;
                        retval = sizeof(udev->brightness);
```

Present since driver introduction (2016).

### Step 6.2: Backport Complications
**Record:** Clean apply expected — surrounding code unchanged since
introduction. No conflicts identified.

### Step 6.3: Related Fixes Already Present?
**Record:** Buffer overread fix (`6dd51d84a9502`) is present. This
`copy_to_user` fix is **not** present (`git log --grep` found no match).

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem
**Record:** `drivers/leds/` — **PERIPHERAL** (optional
`CONFIG_LEDS_USER` module). Not core kernel, but used for
virtual/userspace LEDs and testing.

### Step 7.2: Activity
**Record:** LEDs subsystem actively maintained; recent `uleds` stable
backport in this tree.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users of `/dev/uleds` with `CONFIG_LEDS_USER` enabled
(module or built-in). Enabled in some RISC-V defconfigs
(`nommu_k210_defconfig`, `nommu_k210_sdcard_defconfig`). Driver-
specific, not universal.

### Step 8.2: Trigger Conditions
**Record:** `copy_to_user()` failure — typically invalid/unmapped
userspace buffer. Uncommon with well-behaved apps; possible with signal
interruption edge cases or buggy userspace. Unprivileged users can
trigger via `read()` on `/dev/uleds`.

### Step 8.3: Failure Mode Severity
**Record:**
- Wrong success return (positive byte count instead of `-EFAULT`) —
  **MEDIUM** for API correctness
- Lost brightness update (`new_data` cleared on failure) — **MEDIUM**
  functional data loss
- No kernel crash, oops, memory corruption, or deadlock — not
  **CRITICAL**

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Correct error reporting; preserves pending updates for
  retry; aligns read path with write path and kernel conventions.
- **Risk:** Very low — 6-line change, error-path only.
- **Ratio:** Modest benefit, negligible risk. Appropriate for stable
  given trivial scope and clear correctness bug.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real, verifiable bug (ignored `copy_to_user`, wrong return, lost
  state)
- Obviously correct, minimal fix
- Bug present since 2016 in this 6.18.y tree
- Maintainer sign-off (Lee Jones)
- Consistent with sibling code (`uleds_write`, `uinput.c`)
- Same driver recently received a stable backport (`6dd51d84a9502`)
- Applies cleanly

**AGAINST backport:**
- Niche optional driver (`CONFIG_LEDS_USER`)
- No user reports, syzbot, or crash reports
- `copy_to_user` failure is rare (mostly bad userspace buffers)
- Does not meet highest-severity bar (crash/security/corruption)
- Mailing list review unverified

**UNRESOLVED:**
- Full lore review thread and any explicit stable nomination

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — logic is clear; no `Tested-
   by` but pattern is standard.
2. Fixes a real bug? **PASS** — incorrect return value and lost update
   state.
3. Important issue? **PASS (borderline)** — not crash/security, but
   functional API bug with data loss on error path.
4. Small and contained? **PASS** — ~6 lines, one function.
5. No new features/APIs? **PASS**
6. Can apply to local tree? **PASS** — buggy code confirmed present.

### Step 9.3: Exception Categories
**Record:** None (not device ID, quirk, DT, build, or docs fix).

### Step 9.4: Decision Rationale

For **linux-6.18.y** at 6.18.44: the bug exists, the fix is trivial and
obviously correct, and the functional impact (false success + discarded
brightness update) is a real defect in the userspace interface. While
the driver is peripheral and failures are uncommon, the fix has
essentially zero regression risk and corrects longstanding incorrect
behavior. The same file was recently deemed stable-worthy for a separate
`uleds` fix. This meets stable criteria.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body from user-provided commit
  message
- **[Phase 2]** Diff analysis: 6 lines in `uleds_read()` error path;
  verified against `drivers/leds/uleds.c`
- **[Phase 3]** `git blame -L 150,155`: buggy code from `e381322b0190c`
  (2016)
- **[Phase 3]** `git show e381322b0190c`: original driver had same bug
- **[Phase 3]** `git log --oneline -20 -- drivers/leds/uleds.c`: file
  history reviewed
- **[Phase 3]** `git log --grep="uleds: Return -EFAULT"`: no match — fix
  not in tree
- **[Phase 4]** `b4 dig -c 470015e3f8020`: failed (commit not in tree)
- **[Phase 4]** WebFetch lore/patch.msgid.link: blocked by bot
  protection — **UNVERIFIED** review thread
- **[Phase 5]** Read `uleds_fops`: `.read = uleds_read`
- **[Phase 5]** Grep `uinput.c`: confirmed `-EFAULT` pattern on
  `copy_to_user` failure
- **[Phase 5]** Read `tools/leds/uledmon.c`: userspace consumer of
  `read()` on `/dev/uleds`
- **[Phase 6]** `git describe HEAD`: v6.18.44; `make kernelversion`:
  6.18.44
- **[Phase 6]** Read lines 151–154: buggy code confirmed present
- **[Phase 6]** `git branch`: detached from `stable/linux-6.18.y`
- **[Phase 6]** `git show 6dd51d84a9502`: related stable backport
  present; different bug
- **[Phase 7]** Read `drivers/leds/Kconfig`: `CONFIG_LEDS_USER` optional
  module
- **[Phase 7]** Grep defconfigs: `CONFIG_LEDS_USER=y` in two RISC-V
  configs
- **[Phase 8]** Compared `uleds_write()` lines 97–100: correct `-EFAULT`
  handling on copy failure

**YES**

 drivers/leds/uleds.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/uleds.c b/drivers/leds/uleds.c
index 48ca73c106bc0..8e250838a15e5 100644
--- a/drivers/leds/uleds.c
+++ b/drivers/leds/uleds.c
@@ -148,10 +148,13 @@ static ssize_t uleds_read(struct file *file, char __user *buffer, size_t count,
 		} else if (!udev->new_data && (file->f_flags & O_NONBLOCK)) {
 			retval = -EAGAIN;
 		} else if (udev->new_data) {
-			retval = copy_to_user(buffer, &udev->brightness,
-					      sizeof(udev->brightness));
-			udev->new_data = false;
-			retval = sizeof(udev->brightness);
+			if (copy_to_user(buffer, &udev->brightness,
+					 sizeof(udev->brightness))) {
+				retval = -EFAULT;
+			} else {
+				udev->new_data = false;
+				retval = sizeof(udev->brightness);
+			}
 		}
 
 		mutex_unlock(&udev->mutex);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH AUTOSEL 6.18] leds: tps6131x: Increase overvoltage protection threshold to 6V
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
                   ` (3 preceding siblings ...)
  2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] leds: uleds: Return -EFAULT on copy_to_user() failure Sasha Levin
@ 2026-08-31 13:29 ` Sasha Levin
  2026-08-31 17:13   ` sashiko-bot
  4 siblings, 1 reply; 10+ messages in thread
From: Sasha Levin @ 2026-08-31 13:29 UTC (permalink / raw)
  To: patches, stable
  Cc: Matthias Fend, Lee Jones, Sasha Levin, pavel, linux-leds,
	linux-kernel

From: Matthias Fend <matthias.fend@emfend.at>

[ Upstream commit d43f1d792902ba0a53fd311bff2cf96095c7606d ]

Currently, there may be cases where the overvoltage detection is triggered
even with a valid and generally functioning hardware setup. This occurs,
for example, when the input voltage exceeds the currently used overvoltage
threshold of 4.65V (typical). Since input voltages up to 5V are supported,
the threshold should be adjusted accordingly.

While the target output voltage setting has no effect on the LED operation
used here, it indirectly selects the threshold for overvoltage detection.
Set this to a value of 4.95V to select a threshold of 6V (typical).

Signed-off-by: Matthias Fend <matthias.fend@emfend.at>
Link: https://patch.msgid.link/20260527-leds-tps6131x-ovp-v1-1-1ac70d03c9eb@emfend.at
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `leds: tps6131x: Increase overvoltage
protection threshold to 6V`

**Local tree:** `v6.18.44` (`VERSION = 6`, `PATCHLEVEL = 18`, `SUBLEVEL
= 44`)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject line
**Record:** `[leds: tps6131x]` `[Increase]` — adjust overvoltage
protection (OVP) threshold from ~4.65V to 6V in chip initialization.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Matthias Fend `<matthias.fend@emfend.at>` (driver
  author / hardware vendor contact)
- **Link:** `https://patch.msgid.link/20260527-leds-
  tps6131x-ovp-v1-1-1ac70d03c9eb@emfend.at`
- **Signed-off-by:** Lee Jones `<lee@kernel.org>` (LED subsystem
  maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Cc:
  stable@vger.kernel.org`

Notable: maintainer ack; no fuzzer or user bug reports in the message.

### Step 1.3: Body analysis
**Record:**
- **Bug:** OVP can trip on valid hardware when input voltage exceeds the
  current ~4.65V threshold.
- **Symptom:** Spurious overvoltage protection on systems with input up
  to 5V (within chip spec).
- **Root cause:** `tps6131x_init_chip()` writes REG_6 with only `ENTS`,
  leaving OV field at 0 (~4.65V). The OV setting must be programmed via
  the target-output-voltage field; value `TPS6131X_OV_4950MV` selects a
  6V (typical) threshold.
- **Versions:** Driver landed in v6.17; this tree (6.18.44) includes it.

### Step 1.4: Hidden bug fix?
**Record:** Yes — described as a threshold increase, but it fixes
incorrect register programming in `tps6131x_init_chip()` that leaves OVP
too low for normal 5V operation.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
**Record:**
- **File:** `drivers/leds/flash/leds-tps6131x.c` (+1 effective line
  change in one hunk; whitespace-only elsewhere in hunk)
- **Function:** `tps6131x_init_chip()`
- **Scope:** Single-file, surgical (1 logical line)

### Step 2.2: Code flow change
**Record:**
- **Before:** `val = TPS6131X_REG_6_ENTS;` → `regmap_write(REG_6, 0x80)`
  — only bit 7 set; OV field (bits 0–3) cleared to 0.
- **After:** `val = TPS6131X_REG_6_ENTS | (TPS6131X_OV_4950MV <<
  TPS6131X_REG_6_OV_SHIFT);` — preserves ENTS and sets OV to value 9 (6V
  typical threshold per commit message).
- **Path:** Probe-time chip init, after reset, before LED class setup.

### Step 2.3: Bug mechanism
**Record:** **Logic / hardware configuration bug.** `regmap_write()`
replaces the full register. Writing only `ENTS` clears OV to the lowest
threshold (~4.65V), below the supported 5V input range. This contradicts
`tps6131x_regmap_defaults[]`, which already specifies
`TPS6131X_OV_4950MV` for REG_6.

### Step 2.4: Fix quality
**Record:** Obviously correct — aligns runtime init with existing regmap
defaults and datasheet intent. Minimal change, no API changes, very low
regression risk.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
**Record:** Buggy line `val = TPS6131X_REG_6_ENTS;` introduced in
`b338a2ae9b316` (2025-05-14), “leds: tps6131x: Add support for Texas
Instruments TPS6131X flash LED driver”. Present since driver
introduction in v6.17.

### Step 3.2: Fixes: tag
**Record:** N/A — no `Fixes:` tag. Original buggy commit is
`b338a2ae9b316`, confirmed ancestor of HEAD.

### Step 3.3: Related file history
**Record:** Driver history in this tree:
- `b338a2ae9b316` — driver added
- `c3c38e8001654` — V4L2 dependency fix
No other OVP-related commits. Standalone fix, not part of a series.

### Step 3.4: Author context
**Record:** Matthias Fend authored the original driver and DT binding;
listed as maintainer in
`Documentation/devicetree/bindings/leds/ti,tps61310.yaml`. Lee Jones
committed both driver and this fix.

### Step 3.5: Dependencies
**Record:** None. `TPS6131X_OV_4950MV` and `TPS6131X_REG_6_OV_SHIFT`
already exist in this tree (lines 68–69, 140). Applies standalone.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original discussion
**Record:** Commit not merged in this checkout; `b4 dig -c <hash>` not
usable. `b4 dig` without commitish requires different invocation.
Lore/patch.msgid.link returned 403/bot protection — **could not read
thread**.

### Step 4.2: Reviewers
**Record:** UNVERIFIED — `b4 dig -w` not run (no commitish). Lee Jones
SOB indicates maintainer acceptance.

### Step 4.3: Bug reports
**Record:** No `Reported-by:` or syzbot links. Author-reported hardware
bring-up issue.

### Step 4.4: Related patches
**Record:** Standalone v1 patch per Link message-id
(`...-ovp-v1-1-...`). No series dependency identified.

### Step 4.5: Stable list history
**Record:** UNVERIFIED — lore blocked; no local mbox for this patch.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `tps6131x_init_chip()` modified; callers unchanged.

### Step 5.2: Callers
**Record:** `tps6131x_init_chip()` called once from `tps6131x_probe()`
(line 773), during I2C device probe for `ti,tps61310` / `ti,tps61311`.

### Step 5.3: Callees
**Record:** `regmap_write()` to hardware register REG_6 after
`tps6131x_reset_chip()`.

### Step 5.4: Reachability
**Record:** Triggered at device probe when `CONFIG_LEDS_TPS6131X` is
enabled and hardware is present. Not userspace-syscall reachable, but
affects every boot/probe of this hardware.

### Step 5.5: Similar patterns
**Record:** `tps6131x_regmap_defaults[]` line 156 already uses
`(TPS6131X_OV_4950MV << TPS6131X_REG_6_OV_SHIFT)` for REG_6 — init_chip
was the outlier. `tps6131x_flash_fault_get()` reads REG_6 status flags
but does not program OV.

---

## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE

### Step 6.1: Buggy code present?
**Record:** **Yes.** Current tree at line 280:

```280:282:drivers/leds/flash/leds-tps6131x.c
        val = TPS6131X_REG_6_ENTS;

        ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, val);
```

Driver commit `b338a2ae9b316` is ancestor of HEAD. Bug present since
v6.17.

### Step 6.2: Backport complications
**Record:** Clean apply expected — one-line change, no structural
conflicts. File has low churn since driver addition.

### Step 6.3: Related fixes already present?
**Record:** No — `git log --grep="overvoltage protection threshold"`
returned empty; OVP fix not in tree.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem
**Record:** `drivers/leds/flash/` — LED flash driver for TI TPS6131x.
**Criticality: PERIPHERAL** (specific camera/flash hardware).

### Step 7.2: Activity
**Record:** Driver added recently (6.17); limited follow-up
(`c3c38e8001654` dependency fix only).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who is affected
**Record:** Users of `CONFIG_LEDS_TPS6131X` with TPS6131x hardware on
~5V input rails. No in-tree DTS users found (`grep` for
`tps61310`/`tps6131x` in `*.dts*` returned none), but binding and driver
are present for out-of-tree/custom boards.

### Step 8.2: Trigger conditions
**Record:** Every probe after reset on affected hardware with input
voltage above ~4.65V (common 5V supply). Not timing-dependent;
deterministic misconfiguration.

### Step 8.3: Failure mode severity
**Record:** Spurious hardware overvoltage protection → flash/torch may
fail or report faults on otherwise valid setups. **Severity: MEDIUM** —
real functional failure on affected hardware, not a kernel
oops/panic/data corruption.

### Step 8.4: Risk vs benefit
**Record:**
- **Benefit:** MEDIUM for affected hardware (driver unusable on spec-
  compliant 5V designs without fix)
- **Risk:** VERY LOW (one register bitfield, matches existing defaults
  table)
- **Ratio:** Favorable for a tree that already ships this driver

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Real driver bug present since v6.17 in this 6.18.44 tree
- Incorrect OVP threshold on valid 5V hardware
- One-line, obviously correct fix aligned with regmap defaults
- Hardware configuration / quirk category
- Driver author + subsystem maintainer involvement
- Applies cleanly with no dependencies

**AGAINST backport:**
- Not crash/security/corruption/deadlock
- New, niche driver with no in-tree DTS users yet
- No syzbot or user bug reports
- Lore review thread not accessible for stable nomination confirmation

**Unresolved:** Full mailing-list review discussion; production user
reports.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — matches existing defaults;
   maintainer SOB; logic verified in code
2. Fixes real bug affecting users? **PASS** — spurious OVP on 5V systems
3. Important issue? **PASS (borderline)** — functional hardware failure
   on affected devices; not kernel crash
4. Small and contained? **PASS** — one logical line
5. No new features/APIs? **PASS**
6. Can apply to local tree? **PASS** — driver and symbols exist

### Step 9.3: Exception category
**Record:** Hardware workaround / register quirk — adjusting chip OVP
threshold for correct operation within the 5V input spec.

### Step 9.4: Decision rationale
This tree (`6.18.44`) ships the TPS6131x driver with a probe-time
initialization bug that programs an OVP threshold (~4.65V) below the
chip’s supported 5V input. The fix is minimal, matches values already in
`tps6131x_regmap_defaults[]`, and restores correct hardware behavior for
boards using this driver. While not a kernel crash fix, it is a clear
hardware-configuration bug in a shipped driver — the kind of targeted
quirk fix stable trees routinely carry.

---

## Verification

- **[Phase 1]** `git describe HEAD` → `v6.18.44-1-g2736c32da98b9`;
  Makefile → 6.18.44
- **[Phase 1]** Parsed commit message tags from user query
- **[Phase 2]** Read `drivers/leds/flash/leds-tps6131x.c` lines 63–69,
  140, 156, 254–287, 479–521, 769–775
- **[Phase 2]** Confirmed diff is one logical register-field change in
  `tps6131x_init_chip()`
- **[Phase 3]** `git blame -L 275,285` → buggy line from `b338a2ae9b316`
- **[Phase 3]** `git show b338a2ae9b316` → driver added 2025-05-14,
  regmap default already had `TPS6131X_OV_4950MV`
- **[Phase 3]** `git merge-base --is-ancestor b338a2ae9b316 HEAD` →
  driver in tree
- **[Phase 3]** `git log --oneline -20 -- drivers/leds/flash/` → no OVP
  fix present
- **[Phase 3]** `git log --grep="Increase overvoltage"` → empty (fix not
  merged)
- **[Phase 4]** `b4 dig --help` → requires `-c COMMITISH`; commit not in
  tree
- **[Phase 4]** WebFetch lore + curl raw → 403/bot blocked
- **[Phase 5]** `grep tps6131x_init_chip` → single caller in
  `tps6131x_probe()`
- **[Phase 6]** Current line 280 still `val = TPS6131X_REG_6_ENTS;` (bug
  present)
- **[Phase 6]** `git tag -l 'v6.17*' --contains b338a2ae9b316` → driver
  in v6.17+
- **[Phase 6]** `grep -r tps61310/tps6131x *.dts*` → no in-tree DTS
  users
- **[Phase 7]** Read `Kconfig` `LEDS_TPS6131X` and `MAINTAINERS` entry
- **UNVERIFIED:** Mailing-list reviewer feedback and any `Cc: stable`
  nominations
- **UNVERIFIED:** Whether any production boards in the wild hit this on
  6.18.y yet

**YES**The background search finished: there is **no OVP fix commit** in
this tree yet. `git log --grep` only turned up the original TPS6131x
driver (`b338a2ae9b316`) and related follow-ups (DT binding,
MAINTAINERS, V4L2 dependency) — nothing matching “overvoltage protection
threshold” or OVP changes in `leds-tps6131x.c`.

That matches the earlier analysis: the bug is still present at line 280
(`val = TPS6131X_REG_6_ENTS;`), and backporting this one-line fix to
**6.18.y** is warranted.

**YES**

 drivers/leds/flash/leds-tps6131x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/flash/leds-tps6131x.c b/drivers/leds/flash/leds-tps6131x.c
index 6f4d4fd553610..ad96455b3b02a 100644
--- a/drivers/leds/flash/leds-tps6131x.c
+++ b/drivers/leds/flash/leds-tps6131x.c
@@ -277,7 +277,7 @@ static int tps6131x_init_chip(struct tps6131x *tps6131x)
 	if (ret)
 		return ret;
 
-	val = TPS6131X_REG_6_ENTS;
+	val = TPS6131X_REG_6_ENTS | (TPS6131X_OV_4950MV << TPS6131X_REG_6_OV_SHIFT);
 
 	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, val);
 	if (ret)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH AUTOSEL 6.18] leds: core: Fix race condition for software blink
  2026-08-31 13:23 ` [PATCH AUTOSEL 6.18] leds: core: Fix race condition for software blink Sasha Levin
@ 2026-08-31 14:50   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-31 14:50 UTC (permalink / raw)
  To: Sasha Levin; +Cc: linux-leds, lee

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

New issues:
- [High] Missing `smp_mb__before_atomic()` before `set_bit()` allows the CPU or compiler to reorder the memory write, leading to lost brightness updates.
- [High] The newly added early-return path bypasses max_brightness clamping and state updates, exposing hardware drivers to arbitrary values and causing software state inconsistency.

Pre-existing issues:
- [Critical] A race condition during LED device unregistration can lead to a Use-After-Free of the `led_classdev` structure because background tasks are flushed before the sysfs interface is removed.
- [High] Incomplete fix for software blink race condition leaves a TOCTOU window where brightness changes are permanently lost during `led_stop_software_blink()`.
--

commit 27c103cf78e9f98bb6d9bdb9ae011c1befedc295
Author: Craig McQueen <craig@mcqueen.au>

leds: core: Fix race condition for software blink

led_set_brightness() function: Change handling of software blink to
avoid race conditions when stopping blink and setting brightness.

Triggers may call led_set_brightness(LED_OFF),
led_set_brightness(LED_FULL) in quick succession to disable blinking and
turn the LED on. If the delayed work task has not yet disabled blinking
by the time the second call occurs, then the brightness also needs to be
changed in the delayed work task.

Link: https://patch.msgid.link/20260423113638.2079302-1-craig@mcqueen.au

> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 59473f286b31f..eeb36e50c285c 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -304,24 +304,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
>  
>  void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
>  {
> -	/*
> -	 * If software blink is active, delay brightness setting
> -	 * until the next timer tick.
> -	 */
> -	if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> +	if (brightness) {
>  		/*
> -		 * If we need to disable soft blinking delegate this to the
> -		 * work queue task to avoid problems in case we are called
> -		 * from hard irq context.
> +		 * If software blink disable is pending, also queue brightness setting.
> +		 * If software blink is active, delay brightness setting
> +		 * until the next timer tick.
>  		 */
> -		if (!brightness) {
> -			set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
> +		if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) ||
> +		    test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) {
> +			led_cdev->delayed_set_value = brightness;
> +			set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);

[Severity: High]
Is it possible for the CPU or compiler to reorder this non-atomic write
and the atomic set_bit() operation?

If a worker thread executes test_and_clear_bit(LED_SET_BRIGHTNESS) after
the bit is set but before the data store to delayed_set_value is committed,
could it read a stale delayed_set_value? A subsequent execution of the
worker triggered by the queue_work() would then see the bit cleared and do
nothing, permanently discarding the brightness update.

Would it be better to add an smp_mb__before_atomic() before setting the bit?

[Severity: High]
Does this newly added early return path bypass the clamping of the brightness
value?

By directly storing the unvalidated brightness into delayed_set_value and
returning, this seems to bypass the min(value, led_cdev->max_brightness) check
normally performed in led_set_brightness_nosleep().

If the background worker passes this unvalidated delayed_set_value directly
into the hardware driver's brightness_set callback, could this expose backend
hardware drivers to out-of-bounds values up to ULONG_MAX?

>  			queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
> -		} else {
> -			set_bit(LED_BLINK_BRIGHTNESS_CHANGE,
> -				&led_cdev->work_flags);
> +			return;
> +		} else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
>  			led_cdev->new_blink_brightness = brightness;
> +			set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags);

[Severity: High]
Would this also require an smp_mb__before_atomic() before setting the bit
for the same reordering reasons?

[Severity: High]
This is a pre-existing issue, but does this incomplete fix leave a TOCTOU
window where brightness changes can be permanently lost?

In set_brightness_delayed(), if test_and_clear_bit() clears LED_BLINK_DISABLE
before led_stop_software_blink() completes and clears LED_BLINK_SW, could
a concurrent led_set_brightness(LED_FULL) see LED_BLINK_DISABLE as cleared
but LED_BLINK_SW as still set?

If so, it would fall into this branch, recording the new brightness into
new_blink_brightness and setting LED_BLINK_BRIGHTNESS_CHANGE. But since the
timer is actively being deleted by the worker, it will never fire again,
permanently losing the brightness request.

> +			return;
>  		}

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but is there a use-after-free race condition
during LED device unregistration?

Looking at led_classdev_unregister() in drivers/leds/led-class.c, it appears
flush_work(&led_cdev->set_brightness_work) is called before
device_unregister(led_cdev->dev).

Because device_unregister() is what removes the sysfs attributes, the
brightness sysfs interface remains fully active during and after flush_work().
Could a concurrent sysfs write execute led_set_brightness() and queue the
worker again on the global leds_wq?

When device_unregister() completes and led_cdev is freed, the previously
queued work would execute and access the freed memory. Should this use
cancel_work_sync() after the sysfs node has been fully removed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-161-sashal@kernel.org?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH AUTOSEL 6.18-5.10] leds: pca9532: Don't stop blinking for non-zero brightness
  2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-5.10] leds: pca9532: Don't stop blinking for non-zero brightness Sasha Levin
@ 2026-08-31 14:58   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-31 14:58 UTC (permalink / raw)
  To: Sasha Levin; +Cc: linux-leds, lee

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

Pre-existing issues:
- [High] Race condition in N2100_BEEP input device teardown leading to use-after-free
--

commit 7c75890ff5fc7d7cfe4c161d927be50be43a1827
Author: Tobias Deiminger <tobias.deiminger@linutronix.de>

leds: pca9532: Don't stop blinking for non-zero brightness

This patch prevents the pca9532 driver from incorrectly stopping hardware
blinking when a non-zero brightness value is set. It adds a guard to return 
early if the LED state is PCA9532_PWM1, aligning the behavior with the 
LED subsystem documentation.

Link: https://patch.msgid.link/20260331202848.658676-1-tobias.deiminger@linutronix.de

> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
> index 0344189bb991c..dae7c67605084 100644
> --- a/drivers/leds/leds-pca9532.c
> +++ b/drivers/leds/leds-pca9532.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while looking at this driver,
I noticed a potential use-after-free race during teardown of the N2100_BEEP
input device.

In pca9532_destroy_devices(), the worker is canceled but the managed input
device remains active until devres unwinds:

    case PCA9532_TYPE_N2100_BEEP:
        if (data->idev != NULL) {
            cancel_work_sync(&data->work);
            data->idev = NULL;
        }
        break;

If an input event (e.g., SND_BELL) arrives through the evdev node in this
narrow window before devres unwinds, pca9532_event() will blindly queue
up the work again:

    schedule_work(&data->work);

Once devres unwinding completes, the data structure is freed, but the
newly queued work remains active. When pca9532_input_work() executes, it
could access the freed data:

    static void pca9532_input_work(struct work_struct *work)
    {
        ...
        mutex_lock(&data->update_lock);

Could this result in a use-after-free crash if a user space process writes
to the input event node during driver removal? Should the input device be
explicitly unregistered before canceling the work?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-192-sashal@kernel.org?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH AUTOSEL 6.18-6.12] leds: trigger: gpio: Use GPIOD_FLAGS_BIT_NONEXCLUSIVE
  2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.12] leds: trigger: gpio: Use GPIOD_FLAGS_BIT_NONEXCLUSIVE Sasha Levin
@ 2026-08-31 15:49   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-31 15:49 UTC (permalink / raw)
  To: Sasha Levin; +Cc: lee, linux-leds

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Using GPIOD_FLAGS_BIT_NONEXCLUSIVE causes premature hardware un-configuration and refcount underflow when the shared GPIO is released, breaking the primary owner.
- [High] The patch fails to achieve its stated goal (supporting SFP shared GPIOs) because the SFP driver does not share IRQs, causing activation to always fail with -EBUSY.
- [Medium] The LED trigger driver blindly overwrites the shared GPIO's consumer name, erasing the primary driver's label.
--

commit 02d3bd93d29878c57dc994ff5d3e680adeef7f2e
Author: Piotr Kubik <piotr@kubik.pl>
Subject: leds: trigger: gpio: Use GPIOD_FLAGS_BIT_NONEXCLUSIVE

This commit adds the GPIOD_FLAGS_BIT_NONEXCLUSIVE flag when requesting
the GPIO in the LED trigger driver. This is intended to allow the trigger
to share a GPIO with another driver, such as an SFP module.

Link: https://patch.msgid.link/20260408115106.379834-1-piotr@kubik.pl

> diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c
> index 7f6a2352b0acd..3199275fd250b 100644
> --- a/drivers/leds/trigger/ledtrig-gpio.c
> +++ b/drivers/leds/trigger/ledtrig-gpio.c
> @@ -86,7 +86,8 @@ static int gpio_trig_activate(struct led_classdev *led)
>  	 * The generic property "trigger-sources" is followed,
>  	 * and we hope that this is a GPIO.
>  	 */
> -	gpio_data->gpiod = gpiod_get_optional(dev, "trigger-sources", GPIOD_IN);
> +	gpio_data->gpiod = gpiod_get_optional(dev, "trigger-sources",
> +					      GPIOD_IN | GPIOD_FLAGS_BIT_NONEXCLUSIVE);

[Severity: High]
Does acquiring this GPIO non-exclusively expose the hardware state to being
inadvertently destroyed?

Because gpiolib does not implement reference counting for non-exclusive
consumers, any call to gpiod_put() will execute gpiod_free_commit():

drivers/gpio/gpiolib.c:gpiod_free_commit() {
    ...
    clear_bit(GPIOD_FLAG_REQUESTED, &flags);
    ...
}

This unconditionally frees the GPIO hardware state. Since gpio_trig_activate()
calls gpiod_put() on its error path and during deactivation, won't this tear
down the primary owner's (e.g., the SFP driver's) hardware state?


[Severity: High]
Will this IRQ sharing strategy actually work with the SFP driver?

In gpio_trig_activate(), the LED trigger requests the IRQ with IRQF_SHARED:

    ret = request_threaded_irq(gpiod_to_irq(gpio_data->gpiod), NULL,
                    gpio_trig_irq,
                    IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_RISING
                    | IRQF_TRIGGER_FALLING, "ledtrig-gpio", led);

However, the primary SFP driver requests the same IRQ exclusively:

drivers/net/phy/sfp.c:sfp_probe() {
    ...
    err = devm_request_threaded_irq(sfp->dev, sfp->gpio_irq[i],
                                    NULL, sfp_irq,
                                    IRQF_ONESHOT |
                                    IRQF_TRIGGER_RISING |
                                    IRQF_TRIGGER_FALLING,
                                    sfp_irq_name, sfp);
    ...
}

Since the kernel's IRQ core strictly requires all consumers to use IRQF_SHARED
for sharing to succeed, won't request_threaded_irq() always return -EBUSY? 

If this always fails, it will trigger the error path that calls gpiod_put(),
which leads to the destructive GPIO teardown mentioned above.


[Severity: Medium]
Could this overwrite the original owner's consumer label?

Right after acquiring the GPIO, the trigger driver forcibly sets the
consumer name:

    gpiod_set_consumer_name(gpio_data->gpiod, "led-trigger");

The underlying gpiolib implementation completely overwrites and frees the
old label:

drivers/gpio/gpiolib.c:desc_set_label() {
    ...
    old = rcu_replace_pointer(desc->label, new, 1);
    if (old)
        call_srcu(&desc->gdev->desc_srcu, &old->rh, desc_free_label);
    ...
}

Won't this erase the primary driver's label and corrupt the consumer
tracking for debugging interfaces like debugfs and sysfs? Furthermore, if the
LED trigger fails or deactivates, won't desc_set_label() clear the label
entirely to NULL, permanently masking the primary driver's ownership?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-347-sashal@kernel.org?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH AUTOSEL 6.18] leds: tps6131x: Increase overvoltage protection threshold to 6V
  2026-08-31 13:29 ` [PATCH AUTOSEL 6.18] leds: tps6131x: Increase overvoltage protection threshold to 6V Sasha Levin
@ 2026-08-31 17:13   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-31 17:13 UTC (permalink / raw)
  To: Sasha Levin; +Cc: lee, linux-leds

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

Pre-existing issues:
- [High] Missing cleanup and inverted teardown order for `torch_refresh_work` causes use-after-free.
- [High] The flash strobe is immediately cancelled when turned on due to a missing return statement.
- [High] `led_cdev->max_brightness` is set to milliamperes instead of steps, breaking the standard sysfs brightness scaling interface.
--

commit d25747de0af565ce7fa5d6951483e0ef6ee5bb6d
Author: Matthias Fend <matthias.fend@emfend.at>

leds: tps6131x: Increase overvoltage protection threshold to 6V

This commit increases the overvoltage protection threshold in the tps6131x
driver to 6V. It adjusts the register initialization value to prevent spurious
overvoltage detection on valid 5V hardware setups.

Link: https://patch.msgid.link/20260527-leds-tps6131x-ovp-v1-1-1ac70d03c9eb@emfend.at

> diff --git a/drivers/leds/flash/leds-tps6131x.c b/drivers/leds/flash/leds-tps6131x.c
> index 6f4d4fd553610..ad96455b3b02a 100644
> --- a/drivers/leds/flash/leds-tps6131x.c
> +++ b/drivers/leds/flash/leds-tps6131x.c
> @@ -277,7 +277,7 @@ static int tps6131x_init_chip(struct tps6131x *tps6131x)
>  	if (ret)
>  		return ret;
>  
> -	val = TPS6131X_REG_6_ENTS;
> +	val = TPS6131X_REG_6_ENTS | (TPS6131X_OV_4950MV << TPS6131X_REG_6_OV_SHIFT);
>  
>  	ret = regmap_write(tps6131x->regmap, TPS6131X_REG_6, val);
>  	if (ret)

[ ... ]

> static void tps6131x_remove(struct i2c_client *client)
> {
> 	struct tps6131x *tps6131x = i2c_get_clientdata(client);
> 
> 	v4l2_flash_release(tps6131x->v4l2_flash);
> 
> 	cancel_delayed_work_sync(&tps6131x->torch_refresh_work);
> }

[Severity: High]
This isn't a bug introduced by this patch, but doesn't this teardown order
leave a use-after-free vulnerability in tps6131x_remove()?

Because devm_led_classdev_flash_register_ext() uses devres, the LED device
remains registered when tps6131x_remove() returns. If a user writes to the
LED brightness sysfs attribute during this window, tps6131x_brightness_set()
can reschedule torch_refresh_work. 

The subsequent devres teardown will free the tps6131x struct while the delayed
work timer is still active.

Additionally, in tps6131x_probe():

> 	ret = tps6131x_v4l2_setup(tps6131x);
> 	if (ret)
> 		return dev_err_probe(&client->dev, ret, "Failed to setup v4l2 flash\n");

[Severity: High]
This is a pre-existing issue, but is the timer leaked in this error path?

If tps6131x_v4l2_setup() fails, the probe exits without canceling the delayed
work. Since the LED class device was already registered and could have been
accessed, the timer might still be active.

[ ... ]

> static int tps6131x_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
> {
[ ... ]
> 	if (state) {
> 		ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT,
> 					      TPS6131X_REG_3_SFT, NULL, false, true);
> 		if (ret)
> 			return ret;
> 	}
> 
> 	ret = regmap_update_bits_base(tps6131x->regmap, TPS6131X_REG_3, TPS6131X_REG_3_SFT, 0, NULL,
> 				      false, true);

[Severity: High]
This is a pre-existing issue, but will this immediately terminate the flash
pulse?

If state is true, the driver writes a 1 to TPS6131X_REG_3_SFT, but without
a return statement inside the if-block, execution falls through and writes a
0 to TPS6131X_REG_3_SFT. This appears to break the hardware strobe
functionality by immediately turning it off.

[ ... ]

> static int tps6131x_led_class_setup(struct tps6131x *tps6131x)
> {
[ ... ]
> 	led_cdev->brightness_set_blocking = tps6131x_brightness_set;
> 	led_cdev->max_brightness = tps6131x->max_torch_current_ma;

[Severity: High]
This isn't a bug introduced by this patch, but is max_brightness incorrectly
configured with milliamperes rather than step counts?

In tps6131x_brightness_set(), the brightness parameter is interpreted directly
as the number of steps:

> static int tps6131x_brightness_set(struct led_classdev *cdev, enum led_brightness brightness)
> {
[ ... ]
> 	steps_remaining = (brightness * tps6131x->step_torch_current_ma) / TPS6131X_TORCH_STEP_I_MA;

If max_brightness is exposed as milliamperes (e.g., 175), writing values up
to 175 via sysfs will be processed as 175 steps and clamped to the hardware
maximum. This saturates the computation and locks the output to 100% brightness
for almost the entire exposed sysfs range.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-567-sashal@kernel.org?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH AUTOSEL 6.18-5.10] leds: uleds: Return -EFAULT on copy_to_user() failure
  2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] leds: uleds: Return -EFAULT on copy_to_user() failure Sasha Levin
@ 2026-08-31 17:13   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-31 17:13 UTC (permalink / raw)
  To: Sasha Levin; +Cc: linux-leds, lee

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

Pre-existing issues:
- [High] Missing null-termination check on user-supplied LED name allows out-of-bounds read and information leak.
--

commit a8b47a018dff3b5b2d397104d41c5a68d837fbea
Author: Yousef Alhouseen <alhouseenyousef@gmail.com>

leds: uleds: Return -EFAULT on copy_to_user() failure

uleds_read() copies the current brightness value to userspace but ignores
copy_to_user() failures. It then clears the pending update and reports a
successful full read even when no data was copied. Return -EFAULT when
the copy fails and leave the update pending so a later read can retry.

Link: https://patch.msgid.link/20260521181205.15130-1-alhouseenyousef@gmail.com

> diff --git a/drivers/leds/uleds.c b/drivers/leds/uleds.c
> index 374a841f18c37..ce2ae79d5de04 100644
> --- a/drivers/leds/uleds.c
> +++ b/drivers/leds/uleds.c
> @@ -147,10 +147,13 @@ static ssize_t uleds_read(struct file *file, char __user *buffer, size_t count,
>  		} else if (!udev->new_data && (file->f_flags & O_NONBLOCK)) {
>  			retval = -EAGAIN;
>  		} else if (udev->new_data) {
> -			retval = copy_to_user(buffer, &udev->brightness,
> -					      sizeof(udev->brightness));
> -			udev->new_data = false;
> -			retval = sizeof(udev->brightness);
> +			if (copy_to_user(buffer, &udev->brightness,
> +					 sizeof(udev->brightness))) {
> +				retval = -EFAULT;
> +			} else {
> +				udev->new_data = false;
> +				retval = sizeof(udev->brightness);
> +			}
>  		}
>  
>  		mutex_unlock(&udev->mutex);

While reviewing this patch, I noticed a pre-existing issue in uleds_write()
within the same file:

> static ssize_t uleds_write(struct file *file, const char __user *buffer,
> 			   size_t count, loff_t *ppos)
> {
[ ... ]
> 	if (copy_from_user(&udev->user_dev, buffer,
> 			   sizeof(struct uleds_user_dev))) {
> 		ret = -EFAULT;
> 		goto out;
> 	}
> 
> 	name = udev->user_dev.name;
> 	if (!name[0] || !strcmp(name, ".") || !strcmp(name, "..") ||
> 	    strchr(name, '/')) {

[Severity: High]
This is a pre-existing issue, but does this code lack a null-termination
check on the user-supplied LED name?

If a user writes a payload to /dev/uleds without a null byte in the 64-byte
name array, won't unbounded string functions like strchr() read past the
array boundary?

> 		ret = -EINVAL;
> 		goto out;
> 	}
> 
> 	if (udev->user_dev.max_brightness <= 0) {
> 		ret = -EINVAL;
> 		goto out;
> 	}
> 	udev->led_cdev.max_brightness = udev->user_dev.max_brightness;
> 
> 	ret = devm_led_classdev_register(uleds_misc.this_device,
> 					 &udev->led_cdev);

[Severity: High]
This is a pre-existing issue, but is it possible for
devm_led_classdev_register() to consume the non-null-terminated string to
create the sysfs directory?

If the array is not null-terminated, could this cause an out-of-bounds read
that exposes adjacent kernel heap memory to userspace via the resulting sysfs
directory name, or potentially cause a kernel oops if the read crosses into
an unmapped page?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-563-sashal@kernel.org?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-31 17:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:23 ` [PATCH AUTOSEL 6.18] leds: core: Fix race condition for software blink Sasha Levin
2026-08-31 14:50   ` sashiko-bot
2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-5.10] leds: pca9532: Don't stop blinking for non-zero brightness Sasha Levin
2026-08-31 14:58   ` sashiko-bot
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.12] leds: trigger: gpio: Use GPIOD_FLAGS_BIT_NONEXCLUSIVE Sasha Levin
2026-08-31 15:49   ` sashiko-bot
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] leds: uleds: Return -EFAULT on copy_to_user() failure Sasha Levin
2026-08-31 17:13   ` sashiko-bot
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18] leds: tps6131x: Increase overvoltage protection threshold to 6V Sasha Levin
2026-08-31 17:13   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox