From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 527D326B955; Tue, 8 Apr 2025 12:48:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744116484; cv=none; b=TixNmLeT3DXL7bQPOwuis5Gc+fy2KnDAvtjUuGhBv2JrHTfBxfYKTX8ubdmPjrOGu2DSB1oy8wad6Yp/r4wCd241VDfKHpt4LPBVWngvHctPSkvS0zpCDsTI+YIQj33ootySd4SWQBbCDYT7sv1vghRVg96Cz8YSTR/MezBas/M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744116484; c=relaxed/simple; bh=egKTCV2zle0BUg2AagiApywgH9qsQGTk+1tvL82+hpA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=IzgjKRI8AzDm1VYUI/uEfdagnXcsR9uiDxFNAHY2iEterGuBVyhrSVo1Ry7GNthsGHeo31FYAkwLYow8yQmhKrXTwJmHWp5WLJyB213nZbjwXWA5puN1cf7Vm0X5+8xeRbf384rPZXU/+Qw/yHZWs1GNGobGssK4VZBFgK1MBCg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b=EEOS3gkp; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.b="EEOS3gkp" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 78374C4CEE5; Tue, 8 Apr 2025 12:48:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1744116483; bh=egKTCV2zle0BUg2AagiApywgH9qsQGTk+1tvL82+hpA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EEOS3gkp8rm5bY55XtO0vAcyb0VcaiB87WOyusZEgBTKHoiZXNk7ni+IodbBEylE1 gHLIj8JmC9QqBc7eCfFYUl1ZCFljU1xr9Tkbz0i9rEXEumANt7G/QExVv5HmCS2WJa oYpXYRp2IMCwvIHJEgJJ61X+yHpMYWGwcK7P54cM= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Remi Pommarel , Hans de Goede , Lee Jones , Sasha Levin Subject: [PATCH 6.12 156/423] leds: Fix LED_OFF brightness race Date: Tue, 8 Apr 2025 12:48:02 +0200 Message-ID: <20250408104849.374023317@linuxfoundation.org> X-Mailer: git-send-email 2.49.0 In-Reply-To: <20250408104845.675475678@linuxfoundation.org> References: <20250408104845.675475678@linuxfoundation.org> User-Agent: quilt/0.68 X-stable: review X-Patchwork-Hint: ignore Precedence: bulk X-Mailing-List: stable@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 6.12-stable review patch. If anyone has any objections, please let me know. ------------------ From: Remi Pommarel [ Upstream commit 2c70953b6f535f7698ccbf22c1f5ba26cb6c2816 ] While commit fa15d8c69238 ("leds: Fix set_brightness_delayed() race") successfully forces led_set_brightness() to be called with LED_OFF at least once when switching from blinking to LED on state so that hw-blinking can be disabled, another race remains. Indeed in led_set_brightness(LED_OFF) followed by led_set_brightness(any) scenario the following CPU scheduling can happen: CPU0 CPU1 ---- ---- set_brightness_delayed() { test_and_clear_bit(BRIGHTNESS_OFF) led_set_brightness(LED_OFF) { set_bit(BRIGHTNESS_OFF) queue_work() } led_set_brightness(any) { set_bit(BRIGHTNESS) queue_work() //already queued } test_and_clear_bit(BRIGHTNESS) /* LED set with brightness any */ } /* From previous CPU1 queue_work() */ set_brightness_delayed() { test_and_clear_bit(BRIGHTNESS_OFF) /* LED turned off */ test_and_clear_bit(BRIGHTNESS) /* Clear from previous run, LED remains off */ In that case the led_set_brightness(LED_OFF)/led_set_brightness(any) sequence will be effectively executed in reverse order and LED will remain off. With the introduction of commit 32360bf6a5d4 ("leds: Introduce ordered workqueue for LEDs events instead of system_wq") the race is easier to trigger as sysfs brightness configuration does not wait for set_brightness_delayed() work to finish (flush_work() removal). Use delayed_set_value to optionnally re-configure brightness after a LED_OFF. That way a LED state could be configured more that once but final state will always be as expected. Ensure that delayed_set_value modification is seen before set_bit() using smp_mb__before_atomic(). Fixes: fa15d8c69238 ("leds: Fix set_brightness_delayed() race") Signed-off-by: Remi Pommarel Reviewed-by: Hans de Goede Link: https://lore.kernel.org/r/19c81177059dab7b656c42063958011a8e4d1a66.1740050412.git.repk@triplefau.lt Signed-off-by: Lee Jones Signed-off-by: Sasha Levin --- drivers/leds/led-core.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index 001c290bc07b7..cda0995b16798 100644 --- a/drivers/leds/led-core.c +++ b/drivers/leds/led-core.c @@ -159,8 +159,19 @@ static void set_brightness_delayed(struct work_struct *ws) * before this work item runs once. To make sure this works properly * handle LED_SET_BRIGHTNESS_OFF first. */ - if (test_and_clear_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags)) + if (test_and_clear_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags)) { set_brightness_delayed_set_brightness(led_cdev, LED_OFF); + /* + * The consecutives led_set_brightness(LED_OFF), + * led_set_brightness(LED_FULL) could have been executed out of + * order (LED_FULL first), if the work_flags has been set + * between LED_SET_BRIGHTNESS_OFF and LED_SET_BRIGHTNESS of this + * work. To avoid ending with the LED turned off, turn the LED + * on again. + */ + if (led_cdev->delayed_set_value != LED_OFF) + set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags); + } if (test_and_clear_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags)) set_brightness_delayed_set_brightness(led_cdev, led_cdev->delayed_set_value); @@ -331,10 +342,13 @@ void led_set_brightness_nopm(struct led_classdev *led_cdev, unsigned int value) * change is done immediately afterwards (before the work runs), * it uses a separate work_flag. */ - if (value) { - led_cdev->delayed_set_value = value; + led_cdev->delayed_set_value = value; + /* Ensure delayed_set_value is seen before work_flags modification */ + smp_mb__before_atomic(); + + if (value) set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags); - } else { + else { clear_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags); clear_bit(LED_SET_BLINK, &led_cdev->work_flags); set_bit(LED_SET_BRIGHTNESS_OFF, &led_cdev->work_flags); -- 2.39.5