Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The `event_string` buffer used for wedged uevents is limited to 32 bytes (`WEDGE_STR_LEN`), which is insufficient to hold multiple combined recovery methods. This leads to silent string truncation and a malformed UAPI payload. -- --- Patch [1]: [PATCH v13 1/4] drm: Add DRM_WEDGE_RECOVERY_COLD_RESET recovery method --- commit 091feacf4c7aa58c5985f7e23f67de90d3d5760f Author: Mallesh Koujalagi <[email protected]> drm: Add DRM_WEDGE_RECOVERY_COLD_RESET recovery method Introduce DRM_WEDGE_RECOVERY_COLD_RESET (BIT(4)) recovery method to handle scenarios requiring device power cycle. This method addresses cases where other recovery mechanisms are insufficient to restore device functionality. > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index e51ed959da89f..8519e97ef5d31 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -545,6 +545,8 @@ static const char *drm_get_wedge_recovery(unsigned int > opt) > return "bus-reset"; > case DRM_WEDGE_RECOVERY_VENDOR: > return "vendor-specific"; > + case DRM_WEDGE_RECOVERY_COLD_RESET: > + return "cold-reset"; [Severity: High] This is a pre-existing issue, but will adding the "cold-reset" recovery method cause the event_string buffer to overflow and silently truncate? Looking at drm_dev_wedged_event() in drivers/gpu/drm/drm_drv.c, the event_string buffer is limited to 32 bytes (WEDGE_STR_LEN). If multiple recovery methods are combined, their appended strings can exceed this limit: drivers/gpu/drm/drm_drv.c:drm_dev_wedged_event() { char event_string[WEDGE_STR_LEN]; ... ... len = scnprintf(event_string, sizeof(event_string), "%s", "WEDGED="); for_each_set_bit(opt, &method, BITS_PER_TYPE(method)) { recovery = drm_get_wedge_recovery(opt); ... len += scnprintf(event_string + len, sizeof(event_string) - len, "%s,", recovery); } if (recovery) /* Get rid of trailing comma */ event_string[len - 1] = '\0'; ... } When the string is truncated by scnprintf() to fit the 32-byte buffer, could the trailing comma removal blindly overwrite the last character of the truncated word (e.g. chopping "cold-reset" down to "cold-r")? It appears this might result in a malformed UAPI payload being sent to userspace when multiple recovery methods are combined. > default: > return NULL; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
