[PATCH v5] drm/drv: Convert wedged event string building to seq_buf

[PATCH v5] drm/drv: Convert wedged event string building to seq_buf

[Mallesh Koujalagi]

event_string[] has a fixed size of WEDGE_STR_LEN (32) bytes.
The original scnprintf()-based loop required a manual pre-flight
bounds check.

Replace the manual bookkeeping with seq_buf, which tracks overflow
internally. seq_buf_printf() writes each "method," token into the
buffer.

On overflow, len retains the position of the last
successful write, so the trailing comma is stripped cleanly without
including any partial method name in the uevent payload.

Fixes: b7cf9f4ac1b8 ("drm: Introduce device wedged event")
Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>
---
v2:
- Add proper logic to handle recovery string. (Raag)

v3:
- Convert manual bounds check to seq_buf. (Jani Nikula)
- Use drm_WARN_ONCE() instead of drm_WARN_ON() for overflow. (Raag)

v4:
- Use DECLARE_SEQ_BUF. (Jani)
- Warn overflow at end of loop.

v5:
- Check and warn overflow inside loop. (Raag)
---
 drivers/gpu/drm/drm_drv.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index cb53baa70995..8c0879c336e5 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -36,6 +36,7 @@
 #include <linux/mount.h>
 #include <linux/pseudo_fs.h>
 #include <linux/sched.h>
+#include <linux/seq_buf.h>
 #include <linux/slab.h>
 #include <linux/sprintf.h>
 #include <linux/srcu.h>
@@ -576,27 +577,31 @@ static const char *drm_get_wedge_recovery(unsigned int opt)
 int drm_dev_wedged_event(struct drm_device *dev, unsigned long method,
 			 struct drm_wedge_task_info *info)
 {
-	char event_string[WEDGE_STR_LEN], pid_string[PID_STR_LEN], comm_string[COMM_STR_LEN];
-	char *envp[] = { event_string, NULL, NULL, NULL };
-	const char *recovery = NULL;
-	unsigned int len, opt;
+	DECLARE_SEQ_BUF(event_string, WEDGE_STR_LEN);
+	char pid_string[PID_STR_LEN], comm_string[COMM_STR_LEN];
+	char *envp[4] = { };
+	unsigned int len = 0, opt;
 
-	len = scnprintf(event_string, sizeof(event_string), "%s", "WEDGED=");
+	seq_buf_puts(&event_string, "WEDGED=");
+	envp[0] = event_string.buffer;
 
 	for_each_set_bit(opt, &method, BITS_PER_TYPE(method)) {
-		recovery = drm_get_wedge_recovery(opt);
+		const char *recovery = drm_get_wedge_recovery(opt);
 		if (drm_WARN_ONCE(dev, !recovery, "invalid recovery method %u\n", opt))
 			break;
 
-		len += scnprintf(event_string + len, sizeof(event_string) - len, "%s,", recovery);
+		if (drm_WARN_ON_ONCE(dev, seq_buf_printf(&event_string, "%s,", recovery)))
+			break;
+
+		len = seq_buf_used(&event_string);
 	}
 
-	if (recovery)
-		/* Get rid of trailing comma */
-		event_string[len - 1] = '\0';
+	if (len)
+		/* Strip trailing comma; also discards any partial overflow entry */
+		event_string.buffer[len - 1] = '\0';
 	else
-		/* Caller is unsure about recovery, do the best we can at this point. */
-		snprintf(event_string, sizeof(event_string), "%s", "WEDGED=unknown");
+		/* No complete entry written, do the best we can at this point. */
+		snprintf(event_string.buffer, event_string.size, "%s", "WEDGED=unknown");
 
 	drm_info(dev, "device wedged, %s\n", method == DRM_WEDGE_RECOVERY_NONE ?
 		 "but no recovery needed" : "needs recovery");
-- 
2.48.1

Re: [PATCH v5] drm/drv: Convert wedged event string building to seq_buf

[Raag Jadav]

On Tue, Aug 18, 2026 at 07:06:11PM +0530, Mallesh Koujalagi wrote:
> event_string[] has a fixed size of WEDGE_STR_LEN (32) bytes.
> The original scnprintf()-based loop required a manual pre-flight
> bounds check.
> 
> Replace the manual bookkeeping with seq_buf, which tracks overflow
> internally. seq_buf_printf() writes each "method," token into the
> buffer.
> 
> On overflow, len retains the position of the last
> successful write, so the trailing comma is stripped cleanly without
> including any partial method name in the uevent payload.
> 
> Fixes: b7cf9f4ac1b8 ("drm: Introduce device wedged event")

This actually doesn't fix anything, it just adds a WARN() splat.
So I'm not sure if this is needed here.

> Signed-off-by: Mallesh Koujalagi <mallesh.koujalagi@intel.com>

Assuming this is tested and there are no regressions.

Reviewed-by: Raag Jadav <raag.jadav@intel.com>

Re: [PATCH v5] drm/drv: Convert wedged event string building to seq_buf

[Mallesh, Koujalagi]

[-- Attachment #1: Type: text/plain, Size: 1406 bytes --]


On 19-08-2026 09:58 am, Raag Jadav wrote:
> On Tue, Aug 18, 2026 at 07:06:11PM +0530, Mallesh Koujalagi wrote:
>> event_string[] has a fixed size of WEDGE_STR_LEN (32) bytes.
>> The original scnprintf()-based loop required a manual pre-flight
>> bounds check.
>>
>> Replace the manual bookkeeping with seq_buf, which tracks overflow
>> internally. seq_buf_printf() writes each "method," token into the
>> buffer.
>>
>> On overflow, len retains the position of the last
>> successful write, so the trailing comma is stripped cleanly without
>> including any partial method name in the uevent payload.
>>
>> Fixes: b7cf9f4ac1b8 ("drm: Introduce device wedged event")
> This actually doesn't fix anything, it just adds a WARN() splat.
> So I'm not sure if this is needed here.

In previous implementation multiple recovery methods may exceed 
WEDGE_STR_LEN (32) that

cause silently truncate a recovery method and emit a malformed uevent 
string containing a

partial token, however using seq_buf makes overflow

explicit, when it occurs, emits a WARN and include complete recovery name

in the uevent string so IMO, it's fix.

>
>> Signed-off-by: Mallesh Koujalagi<mallesh.koujalagi@intel.com>
> Assuming this is tested and there are no regressions.
>
> Reviewed-by: Raag Jadav<raag.jadav@intel.com>

Thanks Raag!

The changes are tested and no regressions were observed in my testing.


Thanks,

-/Mallesh

[-- Attachment #2: Type: text/html, Size: 38420 bytes --]

Re: [PATCH v5] drm/drv: Convert wedged event string building to seq_buf

[Raag Jadav]

On Wed, Aug 19, 2026 at 03:48:31PM +0530, Mallesh, Koujalagi wrote:
> On 19-08-2026 09:58 am, Raag Jadav wrote:
> > On Tue, Aug 18, 2026 at 07:06:11PM +0530, Mallesh Koujalagi wrote:
> > > event_string[] has a fixed size of WEDGE_STR_LEN (32) bytes.
> > > The original scnprintf()-based loop required a manual pre-flight
> > > bounds check.
> > > 
> > > Replace the manual bookkeeping with seq_buf, which tracks overflow
> > > internally. seq_buf_printf() writes each "method," token into the
> > > buffer.
> > > 
> > > On overflow, len retains the position of the last
> > > successful write, so the trailing comma is stripped cleanly without
> > > including any partial method name in the uevent payload.
> > > 
> > > Fixes: b7cf9f4ac1b8 ("drm: Introduce device wedged event")
> > This actually doesn't fix anything, it just adds a WARN() splat.
> > So I'm not sure if this is needed here.
> 
> In previous implementation multiple recovery methods may exceed
> WEDGE_STR_LEN (32) that
> 
> cause silently truncate a recovery method and emit a malformed uevent string
> containing a
> 
> partial token, however using seq_buf makes overflow
> 
> explicit, when it occurs, emits a WARN and include complete recovery name
> 
> in the uevent string so IMO, it's fix.

Is there an existing drm_dev_wedged_event() user that is facing this
issue? Please attach bug reports along with reproducers (if any).

Improvements are good, but not everything is a bug because AI said so :)

Raag

Re: [PATCH v5] drm/drv: Convert wedged event string building to seq_buf

[Mallesh, Koujalagi]

On 19-08-2026 04:22 pm, Raag Jadav wrote:
> On Wed, Aug 19, 2026 at 03:48:31PM +0530, Mallesh, Koujalagi wrote:
>> On 19-08-2026 09:58 am, Raag Jadav wrote:
>>> On Tue, Aug 18, 2026 at 07:06:11PM +0530, Mallesh Koujalagi wrote:
>>>> event_string[] has a fixed size of WEDGE_STR_LEN (32) bytes.
>>>> The original scnprintf()-based loop required a manual pre-flight
>>>> bounds check.
>>>>
>>>> Replace the manual bookkeeping with seq_buf, which tracks overflow
>>>> internally. seq_buf_printf() writes each "method," token into the
>>>> buffer.
>>>>
>>>> On overflow, len retains the position of the last
>>>> successful write, so the trailing comma is stripped cleanly without
>>>> including any partial method name in the uevent payload.
>>>>
>>>> Fixes: b7cf9f4ac1b8 ("drm: Introduce device wedged event")
>>> This actually doesn't fix anything, it just adds a WARN() splat.
>>> So I'm not sure if this is needed here.
>> In previous implementation multiple recovery methods may exceed
>> WEDGE_STR_LEN (32) that
>>
>> cause silently truncate a recovery method and emit a malformed uevent string
>> containing a
>>
>> partial token, however using seq_buf makes overflow
>>
>> explicit, when it occurs, emits a WARN and include complete recovery name
>>
>> in the uevent string so IMO, it's fix.
> Is there an existing drm_dev_wedged_event() user that is facing this
> issue? Please attach bug reports along with reproducers (if any).

Currently Sashiko is already reported that issue, here is details:

https://sashiko.dev/#/patchset/20260720101815.526813-5-mallesh.koujalagi%40intel.com

>
> Improvements are good, but not everything is a bug because AI said so :)

Agreed!, however in future, if we support multiple recovery method, 
definitely we encounter such issue.


Thanks,

-/Mallesh

>
> Raag