mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line
@ 2026-10-16  5:27 Devin Wittmayer
  2026-10-16  5:27 ` [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen Devin Wittmayer
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Devin Wittmayer @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Jocelyn Falempe, Javier Martinez Canillas
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, luyuantao, dri-devel, linux-kernel,
	stable

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


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

* [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen
  2026-10-16  5:27 [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line Devin Wittmayer
@ 2026-10-16  5:27 ` Devin Wittmayer
  2026-10-16  7:47   ` Tvrtko Ursulin
  2026-10-16 21:18   ` Jocelyn Falempe
       [not found] ` <sashiko-outbox-173828@kernel.org>
  2026-10-16 21:09 ` Jocelyn Falempe
  2 siblings, 2 replies; 6+ messages in thread
From: Devin Wittmayer @ 2026-10-16  5:27 UTC (permalink / raw)
  To: Jocelyn Falempe, Javier Martinez Canillas
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Tvrtko Ursulin, dri-devel,
	linux-kernel

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


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

* Re: [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen
  2026-10-16  5:27 ` [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen Devin Wittmayer
@ 2026-10-16  7:47   ` Tvrtko Ursulin
  2026-10-16 21:18   ` Jocelyn Falempe
  1 sibling, 0 replies; 6+ messages in thread
From: Tvrtko Ursulin @ 2026-10-16  7:47 UTC (permalink / raw)
  To: Devin Wittmayer, Jocelyn Falempe, Javier Martinez Canillas
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, dri-devel, linux-kernel


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;
>   		}


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

* Re: [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line
       [not found] ` <sashiko-outbox-173828@kernel.org>
@ 2026-10-16 15:29   ` Devin Wittmayer
  0 siblings, 0 replies; 6+ messages in thread
From: Devin Wittmayer @ 2026-10-16 15:29 UTC (permalink / raw)
  To: sashiko-bot
  Cc: Jocelyn Falempe, Javier Martinez Canillas, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel

On Fri, Oct 16, 2026 at 05:39:51AM +0000, sashiko-bot@kernel.org wrote:
> Does this code underflow the pointer arithmetic if kmsg_len is 1?

Yes, when a chunk holds only an empty line. The pointer is then only
compared, never read, and nothing is drawn.

> Does the strict inequality prematurely terminate parsing? It appears it
> would silently drop 1-char lines.

Yes, when the one character line is the first in a chunk. Anywhere else
it is drawn.

> 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.

No, a normal first line is drawn.

Both real cases need printk timestamps off, and both also happen without
this patch. QEMU on drm-misc-fixes, timestamps off, 10 runs each, with
and without this patch:

  normal line at a chunk start         drawn
  one character line at a chunk start  not drawn
  one character line mid chunk         drawn
  chunk of only an empty line          pointer one before the buffer,
                                       nothing drawn

With timestamps on, the one character line was drawn and no chunk held
only an empty line. Every run rebooted.

Devin

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

* Re: [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line
  2026-10-16  5:27 [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line Devin Wittmayer
  2026-10-16  5:27 ` [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen Devin Wittmayer
       [not found] ` <sashiko-outbox-173828@kernel.org>
@ 2026-10-16 21:09 ` Jocelyn Falempe
  2 siblings, 0 replies; 6+ messages in thread
From: Jocelyn Falempe @ 2026-10-16 21:09 UTC (permalink / raw)
  To: Devin Wittmayer, Javier Martinez Canillas
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, luyuantao, dri-devel, linux-kernel,
	stable

On 10/16/26 07:27, Devin Wittmayer wrote:
> 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.

Thanks for investigating this issue.
That looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

I'm sorry, I probably forgot to push [1], so I will push both patches 
next week if no other comments.

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

Best regards,

-- 

Jocelyn
> 
> 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


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

* Re: [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen
  2026-10-16  5:27 ` [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen Devin Wittmayer
  2026-10-16  7:47   ` Tvrtko Ursulin
@ 2026-10-16 21:18   ` Jocelyn Falempe
  1 sibling, 0 replies; 6+ messages in thread
From: Jocelyn Falempe @ 2026-10-16 21:18 UTC (permalink / raw)
  To: Devin Wittmayer, Javier Martinez Canillas
  Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, Tvrtko Ursulin, dri-devel,
	linux-kernel

On 10/16/26 07: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.

Thanks it looks good to me.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>
> 
> 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;
>   		}


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

end of thread, other threads:[~2026-10-16 21:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-16  5:27 [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line Devin Wittmayer
2026-10-16  5:27 ` [PATCH 2/2] drm/panic: Accept a trailing newline in panic_screen Devin Wittmayer
2026-10-16  7:47   ` Tvrtko Ursulin
2026-10-16 21:18   ` Jocelyn Falempe
     [not found] ` <sashiko-outbox-173828@kernel.org>
2026-10-16 15:29   ` [PATCH 1/2] drm/panic: Fix kmsg screen hang on a long log line Devin Wittmayer
2026-10-16 21:09 ` Jocelyn Falempe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®