[PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line

[PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line

[Devin Wittmayer]

The kmsg panic screen reads the log back one chunk at a time. A record
that does not fit in a chunk comes back empty without moving on, so the
screen asks for it again forever and the panic never finishes. Every
oops logs the list of loaded modules, and on my machine that list does
not fit.

Read into a buffer the size printk formats one record into, and stop if
a read still comes back empty.

Fixes: 54034bebb22f ("drm/panic: Add a kmsg panic screen")
Cc: stable@vger.kernel.org
Signed-off-by: Devin Wittmayer <lucid_duck@justthetip.ca>
---
I usually work on Wi-Fi drivers, so this is new ground for me. While I
was testing a fix for a Wi-Fi driver crash, my test machine froze after
the crash where it should have rebooted, and stayed that way until I
power cycled it. Working out why led me here.

To reproduce, with the kmsg screen and panic=10 on a display driver with
panic support, as root:

  printf "%600s\n" "" | tr ' ' x > /dev/kmsg
  echo c > /proc/sysrq-trigger

Before this patch the panic never gets to its reboot.

QEMU, 10 runs each, on drm-misc-fixes and drm-misc-next (bochs) and
6.12.113 (simpledrm):

                                     before    after
  one 600 char line                  hangs     reboots
  same, then 400 short lines         hangs     reboots
  one record of 300 short lines      hangs     reboots
  oops with 24 modules loaded        hangs     reboots
  plain panic                        reboots   reboots

amdgpu, 2 runs each, Arch's source and config, before and after:

  6.18.52, the two commands above    hangs     reboots
  6.18.52, one record of 300 lines   hangs     reboots
  6.18.52, oops                      hangs     reboots
  7.2.6, kmsg screen set, oops       hangs     reboots

luyuantao's patch [1] stops reading once the screen is full, so it only
helps the second row. With both on drm-misc-fixes, every row reboots.

The buffer is shared, like the QR code buffers; a panic only draws from
one CPU. In drm-misc-next this code is in drm_panic_helper.c, where the
same change was tested.

[1] https://lore.kernel.org/all/20260126100732.365-1-luyuantao01@163.com/

 drivers/gpu/drm/drm_panic.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index e576c4791861d..29b1487de7ae0 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -556,7 +556,8 @@ static void draw_panic_screen_kmsg(struct drm_scanout_buffer *sb)
 	const struct font_desc *font = get_default_font(sb->width, sb->height, NULL, NULL);
 	struct drm_rect r_screen = DRM_RECT_INIT(0, 0, sb->width, sb->height);
 	struct kmsg_dump_iter iter;
-	char kmsg_buf[512];
+	/* the size printk formats one record into, too big for the stack */
+	static char kmsg_buf[2048];
 	size_t kmsg_len;
 	struct drm_panic_line line;
 	int yoffset;
@@ -574,6 +575,10 @@ static void draw_panic_screen_kmsg(struct drm_scanout_buffer *sb)
 		char *start;
 		char *end;
 
+		/* a record that does not fit comes back empty, every time */
+		if (!kmsg_len)
+			break;
+
 		/* ignore terminating NUL and newline */
 		start = kmsg_buf + kmsg_len - 2;
 		end = kmsg_buf + kmsg_len - 1;

base-commit: 324fac81f365fc123247f9c8a52cb66ebd93b5e5
-- 
2.56.0

[PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen

[Devin Wittmayer]

Setting the panic screen through sysfs with echo fails with EINVAL,
because the value ends in a newline. Ignore it when matching.

Fixes: e85e9ccf3f84 ("drm/panic: Report invalid or unsupported panic modes")
Signed-off-by: Devin Wittmayer <lucid_duck@justthetip.ca>
---
QEMU, 10 runs each, on drm-misc-fixes and drm-misc-next:

                                     before    after
  write of "user\n"                  refused   takes
  write of "bsod\n"                  refused   refused
  drm.panic_screen=user at boot      takes     takes

amdgpu, 2 runs each, 7.2.6 with Arch's source and config:

  echo user                          refused   takes

 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 29b1487de7ae0..7847b38ba9aab 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -839,7 +839,7 @@ static int drm_panic_type_set(const char *val, const struct kernel_param *kp)
 	unsigned int i;
 
 	for (i = 0; i < ARRAY_SIZE(drm_panic_type_map); i++) {
-		if (!strcmp(val, drm_panic_type_map[i])) {
+		if (sysfs_streq(val, drm_panic_type_map[i])) {
 			drm_panic_type = i;
 			return 0;
 		}
-- 
2.56.0

Re: [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen

[Tvrtko Ursulin]

On 16/10/2026 06:27, Devin Wittmayer wrote:
> Setting the panic screen through sysfs with echo fails with EINVAL,
> because the value ends in a newline. Ignore it when matching.
> 
> Fixes: e85e9ccf3f84 ("drm/panic: Report invalid or unsupported panic modes")
> Signed-off-by: Devin Wittmayer <lucid_duck@justthetip.ca>
> ---
> QEMU, 10 runs each, on drm-misc-fixes and drm-misc-next:
> 
>                                       before    after
>    write of "user\n"                  refused   takes
>    write of "bsod\n"                  refused   refused

Hah funny, becuase before e85e9ccf3f84 it used to take "bsod\n" but 
AFAIR it would silently revert to displaying "user". Because old code 
was this:

static void draw_panic_dispatch(struct drm_scanout_buffer *sb)
{
        if (!strcmp(drm_panic_screen, "kmsg")) {
                draw_panic_static_kmsg(sb);
        } else if (!strcmp(drm_panic_screen, "qr_code")) {
                draw_panic_static_qr_code(sb);
        } else {
                draw_panic_static_user(sb);
        }
}

But it is true that it would also silently accept "user\n" so I guess 
you can call it a partial regression.

Anyway, the fix LGTM.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

>    drm.panic_screen=user at boot      takes     takes
> 
> amdgpu, 2 runs each, 7.2.6 with Arch's source and config:
> 
>    echo user                          refused   takes
> 
>   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 29b1487de7ae0..7847b38ba9aab 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -839,7 +839,7 @@ static int drm_panic_type_set(const char *val, const struct kernel_param *kp)
>   	unsigned int i;
>   
>   	for (i = 0; i < ARRAY_SIZE(drm_panic_type_map); i++) {
> -		if (!strcmp(val, drm_panic_type_map[i])) {
> +		if (sysfs_streq(val, drm_panic_type_map[i])) {
>   			drm_panic_type = i;
>   			return 0;
>   		}

Re: [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/panic: pointer underflow and off-by-one line skipping
--

commit 0e87da3652905d3ac59b3e4a4293f550785dff6f
Author: Devin Wittmayer <lucid_duck@justthetip.ca>

drm/panic: Fix kmsg screen hang on a long log line

This commit increases the size of kmsg_buf and makes it static to avoid 
stack issues. It also adds a check to stop reading if kmsg_len is 0,
preventing an infinite loop when a log record doesn't fit in the buffer.

> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index e576c4791861d..29b1487de7ae0 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
[ ... ]
> @@ -574,6 +575,10 @@ static void draw_panic_screen_kmsg(struct drm_scanout_buffer *sb)
>  		char *start;
>  		char *end;
>  
> +		/* a record that does not fit comes back empty, every time */
> +		if (!kmsg_len)
> +			break;
> +
>  		/* ignore terminating NUL and newline */
>  		start = kmsg_buf + kmsg_len - 2;
>  		end = kmsg_buf + kmsg_len - 1;

[Severity: High]
Does this code underflow the pointer arithmetic if kmsg_len is 1? For
example, an empty printk that just outputs a newline would have a length
of 1, resulting in start pointing to kmsg_buf - 1, which causes undefined
behavior.

Also, looking at the code immediately following this in
draw_panic_screen_kmsg():

    while (start > kmsg_buf && yoffset >= 0) {
        ...

Does the strict inequality prematurely terminate parsing? It appears it
would silently drop 1-char lines. It also seems to skip the first log line of
every kmsg dump chunk, because the loop stops before processing when start
reaches kmsg_buf.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016052722.60051-1-lucid_duck@justthetip.ca?part=1