Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Low] Concurrent modification of the global `scale` module parameter causes a data race. Pre-existing issues: - [High] Incomplete fix for division by zero: lack of upper bounds check on `scale` leads to infinite loops and out-of-bounds memory writes. -- --- Patch [1]: [PATCH v3] drm/log: Fix division by zero when scale module parameter is 0 --- commit bb9f2bc713acbd6b1874a441b5a5e50842c6a233 Author: Shixiong Ou <[email protected]> drm/log: Fix division by zero when scale module parameter is 0 The scale module parameter can be set to 0 via kernel command line, causing a division by zero in the rows/columns calculation. This commit clamps it to 1 once in drm_log_register() to prevent the issue since it is read-only at runtime. > diff --git a/drivers/gpu/drm/clients/drm_log.c > b/drivers/gpu/drm/clients/drm_log.c > index e3e02c84a4cfe..b8289e89ed748 100644 > --- a/drivers/gpu/drm/clients/drm_log.c > +++ b/drivers/gpu/drm/clients/drm_log.c > @@ -421,6 +421,9 @@ void drm_log_register(struct drm_device *dev) > { > struct drm_log *new; > > + if (!scale) > + scale = 1; [Severity: High] This isn't a bug introduced by this patch, but does the lack of an upper bound on the scale parameter leave other issues unresolved? If scale is set to a sufficiently large value (e.g., scale=300 on a 1080p display), scaled_font_w becomes larger than the display width. This causes scanout->columns to evaluate to 0 in drm_log_setup_modeset(): scanout->scaled_font_w = scanout->font->width * scale; scanout->columns = width / scanout->scaled_font_w; When scanout->columns is 0, it causes an infinite loop in drm_log_draw_kmsg_record() since len is never decremented: while (len > scanout->columns) { drm_log_draw_new_line(scanout, s, scanout->columns, prefix_len); s += scanout->columns; len -= scanout->columns; prefix_len = 0; } Additionally, an unvalidated massive scale can cause out-of-bounds memory writes in drm_log_clear_line() due to a massive height calculation: iosys_map_memset(&map, r.y1 * fb->pitches[0], 0, height * fb->pitches[0]); Could we validate an upper bound for scale here as well? [Severity: Low] Does modifying the global module parameter here cause a data race? Since drm_log_register() is called during device initialization, multiple DRM devices probing simultaneously could concurrently check and modify the global scale parameter without synchronization. > + > new = kzalloc_obj(*new); > if (!new) > goto err_warn; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
