A few fixes for drm panic, that I found when writing unit tests with kunit.
Jocelyn Falempe (6): drm/panic: Fix drawing the logo on a small narrow screen drm/panic: Fix overlap between qr code and logo drm/panic: Fix qr_code, ensure vmargin is positive drm/panic: Fix kmsg text drawing rectangle drm/panic: Fix divide by 0 if the screen width < font width drm/panic: Fix 24bit pixel crossing page boundaries
drivers/gpu/drm/drm_panic.c | 60 +++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 6 deletions(-)
base-commit: e4bea919584ff292c9156cf7d641a2ab3cbe27b0
If the logo width is bigger than the framebuffer width, and the height is big enough to hold the logo and the message, it will draw at x coordinate that are higher than the width, and ends up in a corrupted image.
Fixes: 4b570ac2eb54 ("drm/rect: Add drm_rect_overlap()") Signed-off-by: Jocelyn Falempe jfalempe@redhat.com --- drivers/gpu/drm/drm_panic.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 1d6312fa1429..23ba791c6131 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -429,6 +429,9 @@ static void drm_panic_logo_rect(struct drm_rect *rect, const struct font_desc *f static void drm_panic_logo_draw(struct drm_scanout_buffer *sb, struct drm_rect *rect, const struct font_desc *font, u32 fg_color) { + if (rect->x2 > sb->width || rect->y2 > sb->height) + return; + if (logo_mono) drm_panic_blit(sb, rect, logo_mono->data, DIV_ROUND_UP(drm_rect_width(rect), 8), 1, fg_color);
Jocelyn Falempe jfalempe@redhat.com writes:
Hello Jocelyn,
If the logo width is bigger than the framebuffer width, and the height is big enough to hold the logo and the message, it will draw at x coordinate that are higher than the width, and ends up in a corrupted image.
Fixes: 4b570ac2eb54 ("drm/rect: Add drm_rect_overlap()") Signed-off-by: Jocelyn Falempe jfalempe@redhat.com
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
The borders of the qr code was not taken into account to check if it overlap with the logo, leading to the logo being partially covered.
Signed-off-by: Jocelyn Falempe jfalempe@redhat.com --- drivers/gpu/drm/drm_panic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 23ba791c6131..179cbf21f22d 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -749,7 +749,7 @@ static int _draw_panic_static_qr_code(struct drm_scanout_buffer *sb) /* Fill with the background color, and draw text on top */ drm_panic_fill(sb, &r_screen, bg_color);
- if (!drm_rect_overlap(&r_logo, &r_msg) && !drm_rect_overlap(&r_logo, &r_qr)) + if (!drm_rect_overlap(&r_logo, &r_msg) && !drm_rect_overlap(&r_logo, &r_qr_canvas)) drm_panic_logo_draw(sb, &r_logo, font, fg_color);
draw_txt_rectangle(sb, font, panic_msg, panic_msg_lines, true, &r_msg, fg_color);
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH 2/6] drm/panic: Fix overlap between qr code and logo Link: https://lore.kernel.org/stable/20251009122955.562888-3-jfalempe%40redhat.com
Jocelyn Falempe jfalempe@redhat.com writes:
The borders of the qr code was not taken into account to check if it overlap with the logo, leading to the logo being partially covered.
Signed-off-by: Jocelyn Falempe jfalempe@redhat.com
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
Depending on qr_code size and screen size, the vertical margin can be negative, that means there is not enough room to draw the qr_code.
So abort early, to avoid a segfault by trying to draw at negative coordinates.
Fixes: cb5164ac43d0f ("drm/panic: Add a QR code panic screen") Signed-off-by: Jocelyn Falempe jfalempe@redhat.com --- drivers/gpu/drm/drm_panic.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 179cbf21f22d..281bb2dabf81 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -736,7 +736,10 @@ static int _draw_panic_static_qr_code(struct drm_scanout_buffer *sb) pr_debug("QR width %d and scale %d\n", qr_width, scale); r_qr_canvas = DRM_RECT_INIT(0, 0, qr_canvas_width * scale, qr_canvas_width * scale);
- v_margin = (sb->height - drm_rect_height(&r_qr_canvas) - drm_rect_height(&r_msg)) / 5; + v_margin = sb->height - drm_rect_height(&r_qr_canvas) - drm_rect_height(&r_msg); + if (v_margin < 0) + return -ENOSPC; + v_margin /= 5;
drm_rect_translate(&r_qr_canvas, (sb->width - r_qr_canvas.x2) / 2, 2 * v_margin); r_qr = DRM_RECT_INIT(r_qr_canvas.x1 + QR_MARGIN * scale, r_qr_canvas.y1 + QR_MARGIN * scale,
Jocelyn Falempe jfalempe@redhat.com writes:
Depending on qr_code size and screen size, the vertical margin can be negative, that means there is not enough room to draw the qr_code.
So abort early, to avoid a segfault by trying to draw at negative coordinates.
Fixes: cb5164ac43d0f ("drm/panic: Add a QR code panic screen") Signed-off-by: Jocelyn Falempe jfalempe@redhat.com
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
The rectangle height was larger than the screen size. This has no real impact.
Signed-off-by: Jocelyn Falempe jfalempe@redhat.com --- drivers/gpu/drm/drm_panic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 281bb2dabf81..69be9d835ccf 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -480,7 +480,7 @@ static int draw_line_with_wrap(struct drm_scanout_buffer *sb, const struct font_ struct drm_panic_line *line, int yoffset, u32 fg_color) { int chars_per_row = sb->width / font->width; - struct drm_rect r_txt = DRM_RECT_INIT(0, yoffset, sb->width, sb->height); + struct drm_rect r_txt = DRM_RECT_INIT(0, yoffset, sb->width, font->height); struct drm_panic_line line_wrap;
if (line->len > chars_per_row) {
Jocelyn Falempe jfalempe@redhat.com writes:
The rectangle height was larger than the screen size. This has no real impact.
Signed-off-by: Jocelyn Falempe jfalempe@redhat.com
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
In the unlikely case that the screen is tiny, and smaller than the font width, it leads to a divide by 0:
draw_line_with_wrap() chars_per_row = sb->width / font->width = 0 line_wrap.len = line->len % chars_per_row;
This will trigger a divide by 0
Signed-off-by: Jocelyn Falempe jfalempe@redhat.com --- drivers/gpu/drm/drm_panic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index 69be9d835ccf..bc5158683b2b 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -523,7 +523,7 @@ static void draw_panic_static_kmsg(struct drm_scanout_buffer *sb) struct drm_panic_line line; int yoffset;
- if (!font) + if (!font || font->width > sb->width) return;
yoffset = sb->height - font->height - (sb->height % font->height) / 2;
Jocelyn Falempe jfalempe@redhat.com writes:
In the unlikely case that the screen is tiny, and smaller than the font width, it leads to a divide by 0:
draw_line_with_wrap() chars_per_row = sb->width / font->width = 0 line_wrap.len = line->len % chars_per_row;
This will trigger a divide by 0
Signed-off-by: Jocelyn Falempe jfalempe@redhat.com
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
When using page list framebuffer, and using RGB888 format, some pixels can cross the page boundaries, and this case was not handled, leading to writing 1 or 2 bytes on the next virtual address.
Add a check and a specific function to handle this case.
Fixes: c9ff2808790f0 ("drm/panic: Add support to scanout buffer as array of pages") Signed-off-by: Jocelyn Falempe jfalempe@redhat.com --- drivers/gpu/drm/drm_panic.c | 46 +++++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c index bc5158683b2b..d4b6ea42db0f 100644 --- a/drivers/gpu/drm/drm_panic.c +++ b/drivers/gpu/drm/drm_panic.c @@ -174,6 +174,33 @@ static void drm_panic_write_pixel24(void *vaddr, unsigned int offset, u32 color) *p = color & 0xff; }
+/* + * Special case if the pixel crosses page boundaries + */ +static void drm_panic_write_pixel24_xpage(void *vaddr, struct page *next_page, + unsigned int offset, u32 color) +{ + u8 *vaddr2; + u8 *p = vaddr + offset; + + vaddr2 = kmap_local_page_try_from_panic(next_page); + + *p++ = color & 0xff; + color >>= 8; + + if (offset == PAGE_SIZE - 1) + p = vaddr2; + + *p++ = color & 0xff; + color >>= 8; + + if (offset == PAGE_SIZE - 2) + p = vaddr2; + + *p = color & 0xff; + kunmap_local(vaddr2); +} + static void drm_panic_write_pixel32(void *vaddr, unsigned int offset, u32 color) { u32 *p = vaddr + offset; @@ -231,7 +258,14 @@ static void drm_panic_blit_page(struct page **pages, unsigned int dpitch, page = new_page; vaddr = kmap_local_page_try_from_panic(pages[page]); } - if (vaddr) + if (!vaddr) + continue; + + // Special case for 24bit, as a pixel might cross page boundaries + if (cpp == 3 && offset + 3 > PAGE_SIZE) + drm_panic_write_pixel24_xpage(vaddr, pages[page + 1], + offset, fg32); + else drm_panic_write_pixel(vaddr, offset, fg32, cpp); } } @@ -321,7 +355,15 @@ static void drm_panic_fill_page(struct page **pages, unsigned int dpitch, page = new_page; vaddr = kmap_local_page_try_from_panic(pages[page]); } - drm_panic_write_pixel(vaddr, offset, color, cpp); + if (!vaddr) + continue; + + // Special case for 24bit, as a pixel might cross page boundaries + if (cpp == 3 && offset + 3 > PAGE_SIZE) + drm_panic_write_pixel24_xpage(vaddr, pages[page + 1], + offset, color); + else + drm_panic_write_pixel(vaddr, offset, color, cpp); } } if (vaddr)
Jocelyn Falempe jfalempe@redhat.com writes:
When using page list framebuffer, and using RGB888 format, some pixels can cross the page boundaries, and this case was not handled, leading to writing 1 or 2 bytes on the next virtual address.
Add a check and a specific function to handle this case.
Fixes: c9ff2808790f0 ("drm/panic: Add support to scanout buffer as array of pages") Signed-off-by: Jocelyn Falempe jfalempe@redhat.com
Reviewed-by: Javier Martinez Canillas javierm@redhat.com
On 09/10/2025 14:24, Jocelyn Falempe wrote:
A few fixes for drm panic, that I found when writing unit tests with kunit.
Pushed to drm-misc-fixes.
Thanks Javier for your reviews.
Jocelyn Falempe (6): drm/panic: Fix drawing the logo on a small narrow screen drm/panic: Fix overlap between qr code and logo drm/panic: Fix qr_code, ensure vmargin is positive drm/panic: Fix kmsg text drawing rectangle drm/panic: Fix divide by 0 if the screen width < font width drm/panic: Fix 24bit pixel crossing page boundaries
drivers/gpu/drm/drm_panic.c | 60 +++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 6 deletions(-)
base-commit: e4bea919584ff292c9156cf7d641a2ab3cbe27b0
Hi
Am 21.10.25 um 11:35 schrieb Jocelyn Falempe:
On 09/10/2025 14:24, Jocelyn Falempe wrote:
A few fixes for drm panic, that I found when writing unit tests with kunit.
Pushed to drm-misc-fixes.
There are many patches without Fixes tag here. Commits in -fixes should preferably have a Fixes tag to help with backporting. No need to revert, but something to keep in mind for next time.
Best regards Thomas
Thanks Javier for your reviews.
Jocelyn Falempe (6): drm/panic: Fix drawing the logo on a small narrow screen drm/panic: Fix overlap between qr code and logo drm/panic: Fix qr_code, ensure vmargin is positive drm/panic: Fix kmsg text drawing rectangle drm/panic: Fix divide by 0 if the screen width < font width drm/panic: Fix 24bit pixel crossing page boundaries
drivers/gpu/drm/drm_panic.c | 60 +++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 6 deletions(-)
base-commit: e4bea919584ff292c9156cf7d641a2ab3cbe27b0
On 21/10/2025 13:24, Thomas Zimmermann wrote:
Hi
Am 21.10.25 um 11:35 schrieb Jocelyn Falempe:
On 09/10/2025 14:24, Jocelyn Falempe wrote:
A few fixes for drm panic, that I found when writing unit tests with kunit.
Pushed to drm-misc-fixes.
There are many patches without Fixes tag here. Commits in -fixes should preferably have a Fixes tag to help with backporting. No need to revert, but something to keep in mind for next time.
Ok, sorry for that. I'll add it next time.
Best regards,
linux-stable-mirror@lists.linaro.org
-
Javier Martinez Canillas -
Jocelyn Falempe -
kernel test robot -
Thomas Zimmermann