All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0
@ 2026-07-29 10:58 oushixiong1025
  2026-07-29 11:14 ` Shixiong Ou
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: oushixiong1025 @ 2026-07-29 10:58 UTC (permalink / raw)
  To: Jocelyn Falempe
  Cc: Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Shixiong Ou

From: Shixiong Ou <oushixiong@kylinos.cn>

The scale module parameter can be set to 0 via kernel command line or
sysfs. When scale is 0, scaled_font_h and scaled_font_w become 0,
causing a division by zero in the rows/columns calculation.

Introduce a drm_log_scale() helper that returns scale ?: 1, and use it
at all read sites. This avoids a race that a setter-based clamp would
have between param_set_uint() and the subsequent check, where another
CPU could observe scale == 0.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
v1->v2:
   Introduce a drm_log_scale() helper.

 drivers/gpu/drm/clients/drm_log.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
index 294b3be1a6b3..9522c1344123 100644
--- a/drivers/gpu/drm/clients/drm_log.c
+++ b/drivers/gpu/drm/clients/drm_log.c
@@ -29,6 +29,11 @@ static unsigned int scale = 1;
 module_param(scale, uint, 0444);
 MODULE_PARM_DESC(scale, "Integer scaling factor for drm_log, default is 1");
 
+static inline unsigned int drm_log_scale(void)
+{
+	return scale ?: 1;
+}
+
 /**
  * DOC: overview
  *
@@ -76,13 +81,13 @@ static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
 {
 	switch (px_width) {
 	case 2:
-		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, scale, color);
+		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
 		break;
 	case 3:
-		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, scale, color);
+		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
 		break;
 	case 4:
-		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, scale, color);
+		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
 		break;
 	default:
 		WARN_ONCE(1, "Can't blit with pixel width %d\n", px_width);
@@ -216,8 +221,8 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
 		return -ENOMEM;
 	}
 	mode_set->fb = scanout->buffer->fb;
-	scanout->scaled_font_h = scanout->font->height * scale;
-	scanout->scaled_font_w = scanout->font->width * scale;
+	scanout->scaled_font_h = scanout->font->height * drm_log_scale();
+	scanout->scaled_font_w = scanout->font->width * drm_log_scale();
 	scanout->rows = height / scanout->scaled_font_h;
 	scanout->columns = width / scanout->scaled_font_w;
 	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus


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

* Re: [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0
  2026-07-29 10:58 [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0 oushixiong1025
@ 2026-07-29 11:14 ` Shixiong Ou
  2026-07-29 11:19 ` sashiko-bot
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Shixiong Ou @ 2026-07-29 11:14 UTC (permalink / raw)
  To: oushixiong1025, Jocelyn Falempe
  Cc: Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Jani Nikula

Adding to Cc Jani Nikula

On 2026/7/29 18:58, oushixiong1025@163.com wrote:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> The scale module parameter can be set to 0 via kernel command line or
> sysfs. When scale is 0, scaled_font_h and scaled_font_w become 0,
> causing a division by zero in the rows/columns calculation.
>
> Introduce a drm_log_scale() helper that returns scale ?: 1, and use it
> at all read sites. This avoids a race that a setter-based clamp would
> have between param_set_uint() and the subsequent check, where another
> CPU could observe scale == 0.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> v1->v2:
>     Introduce a drm_log_scale() helper.
>
>   drivers/gpu/drm/clients/drm_log.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index 294b3be1a6b3..9522c1344123 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -29,6 +29,11 @@ static unsigned int scale = 1;
>   module_param(scale, uint, 0444);
>   MODULE_PARM_DESC(scale, "Integer scaling factor for drm_log, default is 1");
>   
> +static inline unsigned int drm_log_scale(void)
> +{
> +	return scale ?: 1;
> +}
> +
>   /**
>    * DOC: overview
>    *
> @@ -76,13 +81,13 @@ static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
>   {
>   	switch (px_width) {
>   	case 2:
> -		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>   		break;
>   	case 3:
> -		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>   		break;
>   	case 4:
> -		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>   		break;
>   	default:
>   		WARN_ONCE(1, "Can't blit with pixel width %d\n", px_width);
> @@ -216,8 +221,8 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
>   		return -ENOMEM;
>   	}
>   	mode_set->fb = scanout->buffer->fb;
> -	scanout->scaled_font_h = scanout->font->height * scale;
> -	scanout->scaled_font_w = scanout->font->width * scale;
> +	scanout->scaled_font_h = scanout->font->height * drm_log_scale();
> +	scanout->scaled_font_w = scanout->font->width * drm_log_scale();
>   	scanout->rows = height / scanout->scaled_font_h;
>   	scanout->columns = width / scanout->scaled_font_w;
>   	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);

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

* Re: [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0
  2026-07-29 10:58 [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0 oushixiong1025
  2026-07-29 11:14 ` Shixiong Ou
@ 2026-07-29 11:19 ` sashiko-bot
  2026-07-29 12:01 ` Jani Nikula
  2026-07-29 15:00 ` Jocelyn Falempe
  3 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-29 11:19 UTC (permalink / raw)
  To: oushixiong1025; +Cc: dri-devel

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

Pre-existing issues:
- [High] Unbounded module parameter scale causes integer overflow leading to division by zero.
- [High] TOCTOU race in scale evaluation causes out-of-bounds memory read in blitter.
- [High] Unbounded scale causes scanout->columns to evaluate to 0, creating a fatal infinite loop.
- [High] Unbounded scale causes massive out-of-bounds memset when clearing console lines.
--

--- Patch [1]: [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0 ---
commit e2670bb0ffa13ef9c42c7647657baa7cbc2302c7
Author: Shixiong Ou <oushixiong@kylinos.cn>

drm/log: Fix division by zero when scale module parameter is 0

The scale module parameter can be set to 0 via kernel command line or
sysfs. When scale is 0, scaled_font_h and scaled_font_w become 0,
causing a division by zero in the rows/columns calculation.

Introduce a drm_log_scale() helper that returns scale ?: 1, and use it
at all read sites. This avoids a race that a setter-based clamp would
have between param_set_uint() and the subsequent check, where another
CPU could observe scale == 0.

> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index e3e02c84a4cfe..fc30dfc048ae9 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c

[ ... ]

> @@ -76,13 +81,13 @@ static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
>  {
>  	switch (px_width) {
>  	case 2:
> -		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);

[Severity: High]
This isn't a bug introduced by this patch, but does evaluating the scale
parameter dynamically during rendering create a time-of-check to time-of-use
race?

If the scale parameter is modified concurrently between modeset and rendering:

drm_draw.c:drm_draw_blit16() {
    ...
	for (y = 0; y < height; y++)
		for (x = 0; x < width; x++)
			if (drm_draw_is_pixel_fg(sbuf8, spitch, x / scale, y / scale))
    ...
}

Can the loop execute based on old, larger height bounds while dividing y by a
new, smaller scale, causing the font glyph array indices to drastically exceed
the bounds of the unscaled font source buffer?

>  		break;

[ ... ]

> @@ -213,8 +218,8 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
>  		return -ENOMEM;
>  	}
>  	mode_set->fb = scanout->buffer->fb;
> -	scanout->scaled_font_h = scanout->font->height * scale;
> -	scanout->scaled_font_w = scanout->font->width * scale;
> +	scanout->scaled_font_h = scanout->font->height * drm_log_scale();
> +	scanout->scaled_font_w = scanout->font->width * drm_log_scale();

[Severity: High]
This is a pre-existing issue, but can this multiplication overflow 32-bit
arithmetic if the scale module parameter is set to a very large value?

Because scale is unbounded, a large value like 0x10000000 could cause the
result to wrap exactly to 0.

>  	scanout->rows = height / scanout->scaled_font_h;

[Severity: High]
This isn't a bug introduced by this patch, but if scanout->scaled_font_h
wraps to 0 due to an overflow, does this result in the same division by zero
panic that this patch is attempting to fix?

>  	scanout->columns = width / scanout->scaled_font_w;

[Severity: High]
This is a pre-existing issue, but if an excessively large scale is provided
such that scanout->scaled_font_w is greater than width, does scanout->columns
evaluate to 0?

If columns becomes 0, could this trap the logging thread in an infinite loop
when drawing records?

drm_log.c:drm_log_draw_kmsg_record() {
    ...
    while (len > scanout->columns) {
        ...
        len -= scanout->columns;
    }
    ...
}

If scanout->columns is 0, len will never decrease and the loop will never
terminate.

Additionally, this isn't a bug introduced by this patch, but can an unbounded
scale cause an out-of-bounds memset when clearing console lines?

drm_log.c:drm_log_draw_new_line() {
    ...
    drm_log_clear_line(scanout, 2);
    ...
}

drm_log.c:drm_log_clear_line() {
    ...
    iosys_map_memset(&map, r.y1 * fb->pitches[0], 0, height * fb->pitches[0]);
    ...
}

If the excessively large scale makes scaled_font_h extremely large, could the
attempt to unconditionally clear the first three lines overwrite memory
completely outside the bounds of the framebuffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729105840.865696-1-oushixiong1025@163.com?part=1

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

* Re: [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0
  2026-07-29 10:58 [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0 oushixiong1025
  2026-07-29 11:14 ` Shixiong Ou
  2026-07-29 11:19 ` sashiko-bot
@ 2026-07-29 12:01 ` Jani Nikula
  2026-07-29 15:00 ` Jocelyn Falempe
  3 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2026-07-29 12:01 UTC (permalink / raw)
  To: oushixiong1025, Jocelyn Falempe
  Cc: Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Shixiong Ou

On Wed, 29 Jul 2026, oushixiong1025@163.com wrote:
> From: Shixiong Ou <oushixiong@kylinos.cn>
>
> The scale module parameter can be set to 0 via kernel command line or
> sysfs. When scale is 0, scaled_font_h and scaled_font_w become 0,
> causing a division by zero in the rows/columns calculation.
>
> Introduce a drm_log_scale() helper that returns scale ?: 1, and use it
> at all read sites. This avoids a race that a setter-based clamp would
> have between param_set_uint() and the subsequent check, where another
> CPU could observe scale == 0.
>
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> v1->v2:
>    Introduce a drm_log_scale() helper.
>
>  drivers/gpu/drm/clients/drm_log.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index 294b3be1a6b3..9522c1344123 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -29,6 +29,11 @@ static unsigned int scale = 1;
>  module_param(scale, uint, 0444);
>  MODULE_PARM_DESC(scale, "Integer scaling factor for drm_log, default is 1");
>  
> +static inline unsigned int drm_log_scale(void)

Please don't use "inline" in .c files. The compiler will know better
what to do.

BR,
Jani.

> +{
> +	return scale ?: 1;
> +}
> +
>  /**
>   * DOC: overview
>   *
> @@ -76,13 +81,13 @@ static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
>  {
>  	switch (px_width) {
>  	case 2:
> -		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>  		break;
>  	case 3:
> -		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>  		break;
>  	case 4:
> -		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>  		break;
>  	default:
>  		WARN_ONCE(1, "Can't blit with pixel width %d\n", px_width);
> @@ -216,8 +221,8 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
>  		return -ENOMEM;
>  	}
>  	mode_set->fb = scanout->buffer->fb;
> -	scanout->scaled_font_h = scanout->font->height * scale;
> -	scanout->scaled_font_w = scanout->font->width * scale;
> +	scanout->scaled_font_h = scanout->font->height * drm_log_scale();
> +	scanout->scaled_font_w = scanout->font->width * drm_log_scale();
>  	scanout->rows = height / scanout->scaled_font_h;
>  	scanout->columns = width / scanout->scaled_font_w;
>  	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);

-- 
Jani Nikula, Intel

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

* Re: [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0
  2026-07-29 10:58 [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0 oushixiong1025
                   ` (2 preceding siblings ...)
  2026-07-29 12:01 ` Jani Nikula
@ 2026-07-29 15:00 ` Jocelyn Falempe
  2026-07-29 15:16   ` Jani Nikula
  3 siblings, 1 reply; 6+ messages in thread
From: Jocelyn Falempe @ 2026-07-29 15:00 UTC (permalink / raw)
  To: oushixiong1025
  Cc: Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Shixiong Ou

On 29/07/2026 12:58, oushixiong1025@163.com wrote:
> From: Shixiong Ou <oushixiong@kylinos.cn>
> 
> The scale module parameter can be set to 0 via kernel command line or
> sysfs. When scale is 0, scaled_font_h and scaled_font_w become 0,
> causing a division by zero in the rows/columns calculation.
> 
> Introduce a drm_log_scale() helper that returns scale ?: 1, and use it
> at all read sites. This avoids a race that a setter-based clamp would
> have between param_set_uint() and the subsequent check, where another
> CPU could observe scale == 0.

The scale module parameter is read only (that's the meaning of 0444 in 
module_param()) so it can't be set by sysfs, or change at runtime.

So this check can be done only once in drm_log_register().

Best regards,

-- 

Jocelyn
> 
> Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
> ---
> v1->v2:
>     Introduce a drm_log_scale() helper.
> 
>   drivers/gpu/drm/clients/drm_log.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/clients/drm_log.c b/drivers/gpu/drm/clients/drm_log.c
> index 294b3be1a6b3..9522c1344123 100644
> --- a/drivers/gpu/drm/clients/drm_log.c
> +++ b/drivers/gpu/drm/clients/drm_log.c
> @@ -29,6 +29,11 @@ static unsigned int scale = 1;
>   module_param(scale, uint, 0444);
>   MODULE_PARM_DESC(scale, "Integer scaling factor for drm_log, default is 1");
>   
> +static inline unsigned int drm_log_scale(void)
> +{
> +	return scale ?: 1;
> +}
> +
>   /**
>    * DOC: overview
>    *
> @@ -76,13 +81,13 @@ static void drm_log_blit(struct iosys_map *dst, unsigned int dst_pitch,
>   {
>   	switch (px_width) {
>   	case 2:
> -		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit16(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>   		break;
>   	case 3:
> -		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit24(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>   		break;
>   	case 4:
> -		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, scale, color);
> +		drm_draw_blit32(dst, dst_pitch, src, src_pitch, height, width, drm_log_scale(), color);
>   		break;
>   	default:
>   		WARN_ONCE(1, "Can't blit with pixel width %d\n", px_width);
> @@ -216,8 +221,8 @@ static int drm_log_setup_modeset(struct drm_client_dev *client,
>   		return -ENOMEM;
>   	}
>   	mode_set->fb = scanout->buffer->fb;
> -	scanout->scaled_font_h = scanout->font->height * scale;
> -	scanout->scaled_font_w = scanout->font->width * scale;
> +	scanout->scaled_font_h = scanout->font->height * drm_log_scale();
> +	scanout->scaled_font_w = scanout->font->width * drm_log_scale();
>   	scanout->rows = height / scanout->scaled_font_h;
>   	scanout->columns = width / scanout->scaled_font_w;
>   	scanout->front_color = drm_draw_color_from_xrgb8888(0xffffff, format);


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

* Re: [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0
  2026-07-29 15:00 ` Jocelyn Falempe
@ 2026-07-29 15:16   ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2026-07-29 15:16 UTC (permalink / raw)
  To: Jocelyn Falempe, oushixiong1025
  Cc: Javier Martinez Canillas, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, dri-devel,
	linux-kernel, Shixiong Ou

On Wed, 29 Jul 2026, Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 29/07/2026 12:58, oushixiong1025@163.com wrote:
>> From: Shixiong Ou <oushixiong@kylinos.cn>
>> 
>> The scale module parameter can be set to 0 via kernel command line or
>> sysfs. When scale is 0, scaled_font_h and scaled_font_w become 0,
>> causing a division by zero in the rows/columns calculation.
>> 
>> Introduce a drm_log_scale() helper that returns scale ?: 1, and use it
>> at all read sites. This avoids a race that a setter-based clamp would
>> have between param_set_uint() and the subsequent check, where another
>> CPU could observe scale == 0.
>
> The scale module parameter is read only (that's the meaning of 0444 in 
> module_param()) so it can't be set by sysfs, or change at runtime.
>
> So this check can be done only once in drm_log_register().

Oops, missed this in my review completely, thanks!


-- 
Jani Nikula, Intel

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

end of thread, other threads:[~2026-07-29 15:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-29 10:58 [PATCH v2] drm/log: Fix division by zero when scale module parameter is 0 oushixiong1025
2026-07-29 11:14 ` Shixiong Ou
2026-07-29 11:19 ` sashiko-bot
2026-07-29 12:01 ` Jani Nikula
2026-07-29 15:00 ` Jocelyn Falempe
2026-07-29 15:16   ` Jani Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.