[PATCH] leds: multicolor: limit intensity to max_brightness of LED

[PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Michael Tretter]

According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
intensity should not exceed /sys/class/leds/<led>/max_brightness.

The interface doesn't check the values and higher values may lead to
unexpected color changes if the brightness is changed.

Clamp the intensity value to max_brightness.

Fixes: 55d5d3b46b08 ("leds: multicolor: Introduce a multicolor class definition")
Reported-by: Khalid Talash <ktalash@topcon.com>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
 drivers/leds/led-class-multicolor.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
index fd66d2bdeace..5e0ac6465dc3 100644
--- a/drivers/leds/led-class-multicolor.c
+++ b/drivers/leds/led-class-multicolor.c
@@ -48,6 +48,8 @@ static ssize_t multi_intensity_store(struct device *dev,
 			goto err_out;
 		}
 		offset += nrchars;
+		intensity_value[i] = min(intensity_value[i],
+					 led_cdev->max_brightness);
 	}
 
 	offset++;

---
base-commit: c072629f05d7bca1148ab17690d7922a31423984
change-id: 20260123-leds-multicolor-limit-intensity-51e4bb4d6e1c

Best regards,
-- 
Michael Tretter <m.tretter@pengutronix.de>

Re: (subset) [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Lee Jones]

On Fri, 23 Jan 2026 11:13:24 +0100, Michael Tretter wrote:
> According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> intensity should not exceed /sys/class/leds/<led>/max_brightness.
> 
> The interface doesn't check the values and higher values may lead to
> unexpected color changes if the brightness is changed.
> 
> Clamp the intensity value to max_brightness.
> 
> [...]

Applied, thanks!

[1/1] leds: multicolor: limit intensity to max_brightness of LED
      commit: 129f82752bcecd554936209aac4dbdd888e92224

--
Lee Jones [李琼斯]

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Andy Shevchenko]

On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> intensity should not exceed /sys/class/leds/<led>/max_brightness.
> 
> The interface doesn't check the values and higher values may lead to
> unexpected color changes if the brightness is changed.
> 
> Clamp the intensity value to max_brightness.

This also brings a regression if somebody doesn't care about wrapping around.
It's possible to return an error instead, but still the user space will be
broken (in some rare weird cases).

Again, I care even less about this change, but be always careful,
the main rule in the kernel "We do NOT break user space".

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Lee Jones]

On Thu, 05 Mar 2026, Andy Shevchenko wrote:

> On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> > According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> > intensity should not exceed /sys/class/leds/<led>/max_brightness.
> > 
> > The interface doesn't check the values and higher values may lead to
> > unexpected color changes if the brightness is changed.
> > 
> > Clamp the intensity value to max_brightness.
> 
> This also brings a regression if somebody doesn't care about wrapping around.
> It's possible to return an error instead, but still the user space will be
> broken (in some rare weird cases).
> 
> Again, I care even less about this change, but be always careful,
> the main rule in the kernel "We do NOT break user space".

What are you saying?  I should remove this patch?

-- 
Lee Jones [李琼斯]

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Andy Shevchenko]

On Fri, Mar 06, 2026 at 11:30:55AM +0000, Lee Jones wrote:
> On Thu, 05 Mar 2026, Andy Shevchenko wrote:
> 
> > On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> > > According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> > > intensity should not exceed /sys/class/leds/<led>/max_brightness.
> > > 
> > > The interface doesn't check the values and higher values may lead to
> > > unexpected color changes if the brightness is changed.
> > > 
> > > Clamp the intensity value to max_brightness.
> > 
> > This also brings a regression if somebody doesn't care about wrapping around.
> > It's possible to return an error instead, but still the user space will be
> > broken (in some rare weird cases).
> > 
> > Again, I care even less about this change, but be always careful,
> > the main rule in the kernel "We do NOT break user space".
> 
> What are you saying?  I should remove this patch?

It's up to you. I'm saying that this patch deliberately breaks
*misused*, but working user space. I don't care if this patch
goes in or not. The change seems logical, but it should at least
explain that misused user space won't work anymore.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Lee Jones]

On Thu, 05 Mar 2026, Andy Shevchenko wrote:

> On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> > According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> > intensity should not exceed /sys/class/leds/<led>/max_brightness.
> > 
> > The interface doesn't check the values and higher values may lead to
> > unexpected color changes if the brightness is changed.
> > 
> > Clamp the intensity value to max_brightness.
> 
> This also brings a regression if somebody doesn't care about wrapping around.
> It's possible to return an error instead, but still the user space will be
> broken (in some rare weird cases).
> 
> Again, I care even less about this change, but be always careful,
> the main rule in the kernel "We do NOT break user space".

I'm going to remove this patch for now.

/tmp/next/build/drivers/leds/led-class-multicolor.c: In function 'multi_intensity_store':
/tmp/next/build/include/linux/compiler_types.h:706:45: error: call to '__compiletime_assert_434' declared with attribute error: min(intensity_value[i], led_cdev->max_brightness) signedness error
  706 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |                                             ^
/tmp/next/build/include/linux/compiler_types.h:687:25: note: in definition of macro '__compiletime_assert'
  687 |                         prefix ## suffix();                             \
      |                         ^~~~~~
/tmp/next/build/include/linux/compiler_types.h:706:9: note: in expansion of macro '_compiletime_assert'
  706 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |         ^~~~~~~~~~~~~~~~~~~
/tmp/next/build/include/linux/build_bug.h:40:37: note: in expansion of macro 'compiletime_assert'
   40 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
      |                                     ^~~~~~~~~~~~~~~~~~
/tmp/next/build/include/linux/minmax.h:93:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
   93 |         BUILD_BUG_ON_MSG(!__types_ok(ux, uy),           \
      |         ^~~~~~~~~~~~~~~~
/tmp/next/build/include/linux/minmax.h:98:9: note: in expansion of macro '__careful_cmp_once'
   98 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
      |         ^~~~~~~~~~~~~~~~~~
/tmp/next/build/include/linux/minmax.h:105:25: note: in expansion of macro '__careful_cmp'
  105 | #define min(x, y)       __careful_cmp(min, x, y)
      |                         ^~~~~~~~~~~~~
/tmp/next/build/drivers/leds/led-class-multicolor.c:51:38: note: in expansion of macro 'min'
   51 |                 intensity_value[i] = min(intensity_value[i],
      |               

-- 
Lee Jones [李琼斯]

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Andy Shevchenko]

On Fri, Mar 06, 2026 at 01:55:05PM +0000, Lee Jones wrote:
> On Thu, 05 Mar 2026, Andy Shevchenko wrote:
> > On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> > > According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> > > intensity should not exceed /sys/class/leds/<led>/max_brightness.
> > > 
> > > The interface doesn't check the values and higher values may lead to
> > > unexpected color changes if the brightness is changed.
> > > 
> > > Clamp the intensity value to max_brightness.
> > 
> > This also brings a regression if somebody doesn't care about wrapping around.
> > It's possible to return an error instead, but still the user space will be
> > broken (in some rare weird cases).
> > 
> > Again, I care even less about this change, but be always careful,
> > the main rule in the kernel "We do NOT break user space".
> 
> I'm going to remove this patch for now.

Wow, it was even never compiled!
For sure, please drop it.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Michael Tretter]

On Fri, 06 Mar 2026 17:09:46 +0200, Andy Shevchenko wrote:
> On Fri, Mar 06, 2026 at 01:55:05PM +0000, Lee Jones wrote:
> > On Thu, 05 Mar 2026, Andy Shevchenko wrote:
> > > On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> > > > According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> > > > intensity should not exceed /sys/class/leds/<led>/max_brightness.
> > > > 
> > > > The interface doesn't check the values and higher values may lead to
> > > > unexpected color changes if the brightness is changed.
> > > > 
> > > > Clamp the intensity value to max_brightness.
> > > 
> > > This also brings a regression if somebody doesn't care about wrapping around.
> > > It's possible to return an error instead, but still the user space will be
> > > broken (in some rare weird cases).
> > > 
> > > Again, I care even less about this change, but be always careful,
> > > the main rule in the kernel "We do NOT break user space".
> > 
> > I'm going to remove this patch for now.
> 
> Wow, it was even never compiled!

The compile error depends on the used gcc version. I already sent a
patch for the error, which was also already applied [0].

[0] https://lore.kernel.org/all/177273014112.321702.15159156714467402540.b4-ty@kernel.org/

Michael

> For sure, please drop it.

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Michael Tretter]

On Fri, 06 Mar 2026 13:55:05 +0000, Lee Jones wrote:
> On Thu, 05 Mar 2026, Andy Shevchenko wrote:
> 
> > On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> > > According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> > > intensity should not exceed /sys/class/leds/<led>/max_brightness.
> > > 
> > > The interface doesn't check the values and higher values may lead to
> > > unexpected color changes if the brightness is changed.
> > > 
> > > Clamp the intensity value to max_brightness.
> > 
> > This also brings a regression if somebody doesn't care about wrapping around.
> > It's possible to return an error instead, but still the user space will be
> > broken (in some rare weird cases).

If somebody in user space doesn't care about the wrap around for the
brightness value, the behavior will still differ between systems,
because the value for the wraparound for brightness depends on
max_brightness, which is driver specific and some drivers already
implement clamping on max_brightness.

Handling this in a coherent way at the API level leads to more
consistent behavior between different drivers.

Furthermore, intensity for multicolor LEDs behaves kind of like
brightness for single color LEDs and for these, the documentation
(Documentation/ABI/testing/sysfs-class-led) specifies that

	The [brightness] value is between 0 and
	/sys/class/leds/<led>/max_brightness

and

	Most LEDs don't have hardware brightness support, so will
	just be turned on for non-zero brightness settings.

With a wraparound behavior, writing (max_brightness + 1) to brightness
would lead to a wraparound and turn the LED off, which is against this
specification.

I agree that the behavior of intensity is undefined if it exceeds
max_brightness. Do you have an example of user space (or use case) that
relies on this behavior or is this a hypothetical case?

> > 
> > Again, I care even less about this change, but be always careful,
> > the main rule in the kernel "We do NOT break user space".
> 
> I'm going to remove this patch for now.
> 
> /tmp/next/build/drivers/leds/led-class-multicolor.c: In function 'multi_intensity_store':
> /tmp/next/build/include/linux/compiler_types.h:706:45: error: call to '__compiletime_assert_434' declared with attribute error: min(intensity_value[i], led_cdev->max_brightness) signedness error
>   706 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>       |                                             ^
> /tmp/next/build/include/linux/compiler_types.h:687:25: note: in definition of macro '__compiletime_assert'
>   687 |                         prefix ## suffix();                             \
>       |                         ^~~~~~
> /tmp/next/build/include/linux/compiler_types.h:706:9: note: in expansion of macro '_compiletime_assert'
>   706 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
>       |         ^~~~~~~~~~~~~~~~~~~
> /tmp/next/build/include/linux/build_bug.h:40:37: note: in expansion of macro 'compiletime_assert'
>    40 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
>       |                                     ^~~~~~~~~~~~~~~~~~
> /tmp/next/build/include/linux/minmax.h:93:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
>    93 |         BUILD_BUG_ON_MSG(!__types_ok(ux, uy),           \
>       |         ^~~~~~~~~~~~~~~~
> /tmp/next/build/include/linux/minmax.h:98:9: note: in expansion of macro '__careful_cmp_once'
>    98 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
>       |         ^~~~~~~~~~~~~~~~~~
> /tmp/next/build/include/linux/minmax.h:105:25: note: in expansion of macro '__careful_cmp'
>   105 | #define min(x, y)       __careful_cmp(min, x, y)
>       |                         ^~~~~~~~~~~~~
> /tmp/next/build/drivers/leds/led-class-multicolor.c:51:38: note: in expansion of macro 'min'
>    51 |                 intensity_value[i] = min(intensity_value[i],
>       |               

I'm confused because you already applied [0] my patch that fixes this
exact error.

[0] https://lore.kernel.org/all/177273014112.321702.15159156714467402540.b4-ty@kernel.org/

Michael

Re: [PATCH] leds: multicolor: limit intensity to max_brightness of LED

[Andy Shevchenko]

On Fri, Mar 06, 2026 at 04:29:52PM +0100, Michael Tretter wrote:
> On Fri, 06 Mar 2026 13:55:05 +0000, Lee Jones wrote:
> > On Thu, 05 Mar 2026, Andy Shevchenko wrote:
> > > On Fri, Jan 23, 2026 at 11:13:24AM +0100, Michael Tretter wrote:
> > > > According to Documentation/ABI/testing/sysfs-class-led-multicolor, the
> > > > intensity should not exceed /sys/class/leds/<led>/max_brightness.
> > > > 
> > > > The interface doesn't check the values and higher values may lead to
> > > > unexpected color changes if the brightness is changed.
> > > > 
> > > > Clamp the intensity value to max_brightness.
> > > 
> > > This also brings a regression if somebody doesn't care about wrapping around.
> > > It's possible to return an error instead, but still the user space will be
> > > broken (in some rare weird cases).
> 
> If somebody in user space doesn't care about the wrap around for the
> brightness value, the behavior will still differ between systems,
> because the value for the wraparound for brightness depends on
> max_brightness, which is driver specific and some drivers already
> implement clamping on max_brightness.
> 
> Handling this in a coherent way at the API level leads to more
> consistent behavior between different drivers.
> 
> Furthermore, intensity for multicolor LEDs behaves kind of like
> brightness for single color LEDs and for these, the documentation
> (Documentation/ABI/testing/sysfs-class-led) specifies that
> 
> 	The [brightness] value is between 0 and
> 	/sys/class/leds/<led>/max_brightness
> 
> and
> 
> 	Most LEDs don't have hardware brightness support, so will
> 	just be turned on for non-zero brightness settings.
> 
> With a wraparound behavior, writing (max_brightness + 1) to brightness
> would lead to a wraparound and turn the LED off, which is against this
> specification.
> 
> I agree that the behavior of intensity is undefined if it exceeds
> max_brightness. Do you have an example of user space (or use case) that
> relies on this behavior or is this a hypothetical case?

It does not mean if it's hypothetical or real for a (censored) user.
This is an ABI change patch. It might break someone's use case.
At bare minimum the commit message should explain that.

> > > Again, I care even less about this change, but be always careful,
> > > the main rule in the kernel "We do NOT break user space".


-- 
With Best Regards,
Andy Shevchenko