drm/mipi-dbi: clamp the flushed rectangle to the panels fixed mode - #7643
Conversation
mipi_dbi_fb_dirty() addresses the panel controller using the plane source rectangle origin subtracted from the damage rectangle (added in this branch's earlier honour-the-plane-source-offset change, PR raspberrypi#7589). That subtraction assumed the result would already be panel-sized. The damage rectangle from drm_atomic_helper_damage_merged() is clipped against the plane source rectangle's exact 16.16 fixed-point bounds, while the origin subtracted here is that same value truncated to whole pixels. When the origin has a fractional part, the truncation can leave the rectangle's far edge up to a pixel past where a whole-pixel origin would place the panel's own width/height - and tx_buf is sized for exactly the panel, with no slack for that overshoot: a heap buffer overflow. Clamp the rectangle to the panel's fixed mode before using it for the window address or the transfer length. A damage clip can also lie entirely within that one-pixel sliver past the panel edge - the damage iterator clips only against the exact fixed-point bound, not the whole-pixel one used here - collapsing the clamped rectangle to zero width or height. Skip the flush in that case rather than program an inverted (start past end) address window, which is undefined behaviour per the MIPI DCS spec. Found by automated review on the equivalent upstream submission (drm/mipi-dbi: display a cropped region of an oversized framebuffer, dri-devel), across two review rounds; folding both fixes back here since this code has been merged and in use on rpi-7.2.y since PR raspberrypi#7589, ahead of upstream landing it. Signed-off-by: Jonathan Frazin <frazinjonathan@gmail.com>
ef98163 to
2deb0b6
Compare
st7735r flushes through the shared drm_mipi_dbi_plane_helper_atomic_update(), same as the six drm/tiny drivers PR raspberrypi#7589 already raised mode_config.max_width/max_height on - but it lives under drivers/gpu/drm/sitronix/ rather than drivers/gpu/drm/tiny/ and was missed from that original sweep. Raise its limits the same way, to DRM_SHADOW_PLANE_MAX_WIDTH/HEIGHT. Confirmed by grepping the whole tree for DRM_MIPI_DBI_PLANE_HELPER_FUNCS that no other driver shares this flush path and is still missing this. Found by automated review on the equivalent upstream submission (drm/tiny: allow a framebuffer larger than the panel on MIPI DBI drivers, dri-devel, v3 about to be sent); folding the same fix back here for the same reason as the sibling commit on this branch. Signed-off-by: Jonathan Frazin <frazinjonathan@gmail.com>
|
@6by9 hello again. AI found some issues with my dri-devel patches but have corrected those and submitted again upstream. If it gets accepted, not sure this will be needed, but who knows when/if. |
6by9
left a comment
6by9
left a comment
There was a problem hiding this comment.
These do what is intended.
Upstream may decide that your comment in the first patch is too verbose, but so be it.
I had gone looking for the patches the other day and noted no responses. They may have held off because saisiko raised issues, or no one may be interested.
If you're sending a v2 then make a comment in the cover letter that we'd be prepared to pick up maintainership if desired.
|
@6by9 Thanks, I actually send a v2 and v3 with these changes after Sashiko bot had found the issues. V3 has no response from Sashiko so i assume all is good. I could add the note about maintainership now, but I assume the etiquette is to wait for their response to this first, right?. Actually i have no idea what the etiquette is here, seems like there's alot of unwritten rules, like being too verbose. |
|
I'd missed your later revisions - I have email rules to filter the mailing list traffic out into folders, but really ought to make them smarter so that ones which I'm in the to or cc get flagged. The rules are often unwritten sadly. Device tree seems to be the worst for it. And each subsystem can have different requirements, so it can be a case of monitoring the list for a while to pick up the standard complaints. All a little trying. Give it a week, and then give the cover email a ping with a comment on maintainership. |
|
@6by9 that sounds good, I'll wait and see and try and learn the ways of dri-devel, def interested in co-maintaining, that would be very cool. I'm still learning the etiquette of github too, my real life job has nothing to do with this and I have no formal education in this. |
3 participants
Follow-up to #7589. An automated review on the equivalent upstream dri-devel submission ("drm/mipi-dbi: display a cropped region of an oversized framebuffer") caught a heap buffer overflow in mipi_dbi_fb_dirty(): the plane source origin subtracted when addressing the panel controller is truncated to whole pixels, but the damage rectangle is clipped against that origin's exact 16.16 fixed-point value. A fractional origin can leave the rectangle's far edge up to a pixel past the panel's own width/height, overflowing the panel-sized tx_buf.
This has been merged and in use on rpi-7.2.y since #7589, ahead of the upstream submission landing, so folding the same fix back here rather than waiting on that (currently 2 weeks with no maintainer response upstream).
v2 of the upstream series (with this same fix) was just sent to dri-devel.