drivers/gpu/drm/i915/display/intel_backlight.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
The scale() functions detects invalid parameters, but continues
its calculations anyway. This causes bad results if negative values
are used for unsigned operations. Worst case, a division by 0 error
will be seen if source_min == source_max.
On top of that, after v6.13, the sequence of WARN_ON() followed by clamp()
may result in a build error with gcc 13.x.
drivers/gpu/drm/i915/display/intel_backlight.c: In function 'scale':
include/linux/compiler_types.h:542:45: error:
call to '__compiletime_assert_415' declared with attribute error:
clamp() low limit source_min greater than high limit source_max
This happens if the compiler decides to rearrange the code as follows.
if (source_min > source_max) {
WARN(..);
/* Do the clamp() knowing that source_min > source_max */
source_val = clamp(source_val, source_min, source_max);
} else {
/* Do the clamp knowing that source_min <= source_max */
source_val = clamp(source_val, source_min, source_max);
}
Fix the problem by evaluating the return values from WARN_ON and returning
immediately after a warning. While at it, fix divide by zero error seen
if source_min == source_max.
Analyzed-by: Linus Torvalds <torvalds@linux-foundation.org>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Suggested-by: David Laight <david.laight.linux@gmail.com>
Cc: David Laight <david.laight.linux@gmail.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Simplify code to always return target_min after a warning,
and also warn if source_min == source_max.
drivers/gpu/drm/i915/display/intel_backlight.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
index 3f81a726cc7d..ca588bed82b9 100644
--- a/drivers/gpu/drm/i915/display/intel_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_backlight.c
@@ -40,8 +40,9 @@ static u32 scale(u32 source_val,
{
u64 target_val;
- WARN_ON(source_min > source_max);
- WARN_ON(target_min > target_max);
+ if (WARN_ON(source_min >= source_max) ||
+ WARN_ON(target_min > target_max))
+ return target_min;
/* defensive */
source_val = clamp(source_val, source_min, source_max);
--
2.45.2On Tue, Jan 21, 2025 at 06:52:03AM -0800, Guenter Roeck wrote:
> The scale() functions detects invalid parameters, but continues
> its calculations anyway. This causes bad results if negative values
> are used for unsigned operations. Worst case, a division by 0 error
> will be seen if source_min == source_max.
>
> On top of that, after v6.13, the sequence of WARN_ON() followed by clamp()
> may result in a build error with gcc 13.x.
>
> drivers/gpu/drm/i915/display/intel_backlight.c: In function 'scale':
> include/linux/compiler_types.h:542:45: error:
> call to '__compiletime_assert_415' declared with attribute error:
> clamp() low limit source_min greater than high limit source_max
>
> This happens if the compiler decides to rearrange the code as follows.
>
> if (source_min > source_max) {
> WARN(..);
> /* Do the clamp() knowing that source_min > source_max */
> source_val = clamp(source_val, source_min, source_max);
> } else {
> /* Do the clamp knowing that source_min <= source_max */
> source_val = clamp(source_val, source_min, source_max);
> }
>
> Fix the problem by evaluating the return values from WARN_ON and returning
> immediately after a warning. While at it, fix divide by zero error seen
> if source_min == source_max.
>
> Analyzed-by: Linus Torvalds <torvalds@linux-foundation.org>
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Suggested-by: David Laight <david.laight.linux@gmail.com>
> Cc: David Laight <david.laight.linux@gmail.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
I'm pushing this soon to drm-intel-next, unless Linus want to take
this one directly to his tree
Thanks,
Rodrigo.
> ---
> v2: Simplify code to always return target_min after a warning,
> and also warn if source_min == source_max.
>
> drivers/gpu/drm/i915/display/intel_backlight.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_backlight.c b/drivers/gpu/drm/i915/display/intel_backlight.c
> index 3f81a726cc7d..ca588bed82b9 100644
> --- a/drivers/gpu/drm/i915/display/intel_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_backlight.c
> @@ -40,8 +40,9 @@ static u32 scale(u32 source_val,
> {
> u64 target_val;
>
> - WARN_ON(source_min > source_max);
> - WARN_ON(target_min > target_max);
> + if (WARN_ON(source_min >= source_max) ||
> + WARN_ON(target_min > target_max))
> + return target_min;
>
> /* defensive */
> source_val = clamp(source_val, source_min, source_max);
> --
> 2.45.2
>
On Tue, 21 Jan 2025 at 14:59, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>
> I'm pushing this soon to drm-intel-next, unless Linus want to take
> this one directly to his tree
Let's just go through the proper channels and go through the drm tree.
Unless I've issed something, I think this is only an active issue on
parisc (and only with certain compiler versions at that), so it isn't
some super-urgent thing that needs to be expedited.
Thanks,
Linus
On Tue, 21 Jan 2025 15:15:09 -0800 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, 21 Jan 2025 at 14:59, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > > > I'm pushing this soon to drm-intel-next, unless Linus want to take > > this one directly to his tree > > Let's just go through the proper channels and go through the drm tree. > > Unless I've issed something, I think this is only an active issue on > parisc (and only with certain compiler versions at that), so it isn't > some super-urgent thing that needs to be expedited. It probably wants pushing into rc-2. The build bot is complaining again. David
On 2/2/25 05:27, David Laight wrote: > On Tue, 21 Jan 2025 15:15:09 -0800 > Linus Torvalds <torvalds@linux-foundation.org> wrote: > >> On Tue, 21 Jan 2025 at 14:59, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: >>> >>> I'm pushing this soon to drm-intel-next, unless Linus want to take >>> this one directly to his tree >> >> Let's just go through the proper channels and go through the drm tree. >> >> Unless I've issed something, I think this is only an active issue on >> parisc (and only with certain compiler versions at that), so it isn't >> some super-urgent thing that needs to be expedited. > > It probably wants pushing into rc-2. > The build bot is complaining again. > The patch didn't make it into linux-next (as of next-20250131). I assume it either got lost or it was dropped for some reason. Guenter
On Sun, Feb 02, 2025 at 06:28:08AM -0800, Guenter Roeck wrote: > On 2/2/25 05:27, David Laight wrote: > > On Tue, 21 Jan 2025 15:15:09 -0800 > > Linus Torvalds <torvalds@linux-foundation.org> wrote: > > > > > On Tue, 21 Jan 2025 at 14:59, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > > > > > > > > I'm pushing this soon to drm-intel-next, unless Linus want to take > > > > this one directly to his tree > > > > > > Let's just go through the proper channels and go through the drm tree. > > > > > > Unless I've issed something, I think this is only an active issue on > > > parisc (and only with certain compiler versions at that), so it isn't > > > some super-urgent thing that needs to be expedited. > > > > It probably wants pushing into rc-2. > > The build bot is complaining again. > > > > The patch didn't make it into linux-next (as of next-20250131). > I assume it either got lost or it was dropped for some reason. I'm sorry for missing that for our -rc1 cycle. It is now queued for -rc2 > > Guenter >
© 2016 - 2025 Red Hat, Inc.