Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Unconditional 8-line page iterations in `st7571_transform_xy` cause an out-of-bounds read when `nlines` is not a multiple of 8. - [High] Mismatch between framebuffer pitch during conversion and reading causes display corruption and invalid reads from `hwbuf`. -- --- Patch [23]: [PATCH 23/70] drm/sitronix: st7571: Switch to drm_atomic_helper_crtc_create_state --- commit cf528fe94124a1e49e93edb3680754d3976e2864 Author: Maxime Ripard <[email protected]> drm/sitronix: st7571: Switch to drm_atomic_helper_crtc_create_state The st7571 crtc implementation uses the deprecated drm_atomic_helper_crtc_reset() as its reset hook. Switch to drm_atomic_helper_crtc_create_state() instead. > diff --git a/drivers/gpu/drm/sitronix/st7571.c > b/drivers/gpu/drm/sitronix/st7571.c > index 20954c33eca93..fc3ba5e8600f6 100644 > --- a/drivers/gpu/drm/sitronix/st7571.c > +++ b/drivers/gpu/drm/sitronix/st7571.c > @@ -424,7 +424,7 @@ static const struct drm_crtc_helper_funcs > st7571_crtc_helper_funcs = { > }; > > static const struct drm_crtc_funcs st7571_crtc_funcs = { > - .reset = drm_atomic_helper_crtc_reset, > + .atomic_create_state = drm_atomic_helper_crtc_create_state, > .destroy = drm_crtc_cleanup, > .set_config = drm_atomic_helper_set_config, > .page_flip = drm_atomic_helper_page_flip, [Severity: High] This isn't a bug introduced by this patch, but does this code cause an out-of-bounds read when nlines is not a multiple of 8? In drivers/gpu/drm/sitronix/st7571.c:st7571_probe(): st7571->hwbuf = devm_kzalloc(st7571->dev, (st7571->nlines * st7571->ncols * st7571->bpp) / 8, GFP_KERNEL); If nlines is not a multiple of 8, hwbuf is allocated strictly based on the unaligned nlines value. During a display update: drivers/gpu/drm/sitronix/st7571.c:st7571_fb_update_rect_monochrome(): for (int y = rect->y1; y < rect->y2; y += ST7571_PAGE_HEIGHT) { drivers/gpu/drm/sitronix/st7571.c:st7571_transform_xy(): for (int i = 0; i < 8; i++) { int row_idx = y + i; u8 byte = p[row_idx * row_len + x]; This iterates 8 times vertically, which could read beyond the allocated buffer if nlines is smaller than the assumed 8-line page height. [Severity: High] This is a pre-existing issue, but does a mismatch between the framebuffer pitch during conversion and reading cause invalid reads from hwbuf? In drivers/gpu/drm/sitronix/st7571.c:st7571_prepare_buffer_monochrome(): dst_pitch = DIV_ROUND_UP(drm_rect_width(rect), 8); iosys_map_set_vaddr(&dst, st7571->hwbuf); drm_fb_xrgb8888_to_mono(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state); The converted partial damage is tightly packed at offset 0 of hwbuf with a dynamic pitch. However, when reading the data: drivers/gpu/drm/sitronix/st7571.c:st7571_fb_update_rect_monochrome(): row[x] = st7571_transform_xy(st7571->hwbuf, x, y, 1); drivers/gpu/drm/sitronix/st7571.c:st7571_transform_xy(): u8 row_len = 16 * bpp; ... int row_idx = y + i; u8 byte = p[row_idx * row_len + x]; st7571_transform_xy() accesses hwbuf using absolute coordinates and assumes a full-screen layout with a fixed pitch, which seems to completely miss the packed damage data. -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=23
