Re: [PATCH v3] staging: sm750fb: fix mono image source stride mismatch in lynxfb_ops_imageblit()

[Muhammad Bilal]

Sent v4, rebased on current staging-testing and applies cleanly:

Link: https://lore.kernel.org/all/20260901113031.161610-1-meatuni001@xxxxxxxxx/

Thanks,
Bilal


On Tue, Sep 1, 2026 at 3:15 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Jul 28, 2026 at 06:10:50PM +0500, Muhammad Bilal wrote:
> > sm750_hw_imageblit() advances its monochrome source pointer by
> > src_delta per scanline, and computes the correct rounded-up stride
> > internally as:
> >
> >         bytes_per_scan = (width + start_bit + 7) / 8;
> >
> > Its only caller, lynxfb_ops_imageblit(), instead passed src_delta as
> > image->width >> 3. For widths not a multiple of 8 this under-counted
> > the stride, so the source pointer fell further behind the real
> > per-scanline layout on every line, corrupting the rendered image.
> >
> > Rather than just fixing the caller's calculation, remove src_delta
> > as a parameter entirely and have sm750_hw_imageblit() advance by the
> > bytes_per_scan it already computes for itself. There has only ever
> > been one caller, and that caller was passing an out-of-sync
> > derivative of the same width/start_bit values sm750_hw_imageblit()
> > already has, so keeping stride as a separate parameter served no
> > purpose beyond letting the two calculations drift apart, which is
> > exactly what happened here.
> >
> > Rounding up, rather than down, is the direction consistent with the
> > rest of the fbdev core: struct fb_image mono bitmap data (the same
> > image->data this driver receives) is walked elsewhere with byte
> > strides derived from a ceiling division of width by 8. The generic
> > mono bit iterator in drivers/video/fbdev/core/fb_imageblit.h advances
> > scanlines with "iter->data += BITS_TO_BYTES(iter->width)", and
> > BITS_TO_BYTES() (include/linux/bitops.h) is a ceiling division.
> > sm750_hw_imageblit()'s own "(width + start_bit + 7) / 8" is that same
> > ceiling division with an added start_bit offset, so the caller's
> > ">> 3" (floor) was the one calculation out of step with how this data
> > layout is handled everywhere else.
> >
> > Found by code review of sm750_hw_imageblit()'s internal stride
> > calculation against what its only caller was passing in, and
> > confirmed with a clean -Werror build. I do not have this hardware,
> > so this has not been exercised at runtime on real sm750 silicon.
> >
> > Fixes: 81dee67e215b2 ("staging: sm750fb: add sm750 to staging")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Muhammad Bilal <meatuni001@xxxxxxxxx>
> > ---
> > v3: Added the version notes below, which Dan pointed out were
> >     missing from v2. Added Cc: stable@xxxxxxxxxxxxxxx, flagged by
> >     Greg's patch-bot since the Fixes: tag targets an old (2015)
> >     commit. Added the rounding-direction justification Dan asked
> >     about, citing fb_imageblit.h's BITS_TO_BYTES() as the existing
> >     kernel-wide convention for this same struct fb_image layout.
> >     Moved the "no hardware" disclosure from an email reply into the
> >     commit message itself, per Dan's request.
> > v2: Added a Fixes tag. Moved the src_delta calculation into
> >     sm750_hw_imageblit() itself and dropped it as a parameter, per
> >     Dan Carpenter's review.
> >
> >  drivers/staging/sm750fb/sm750.c       | 2 +-
> >  drivers/staging/sm750fb/sm750.h       | 2 +-
> >  drivers/staging/sm750fb/sm750_accel.c | 6 ++----
> >  drivers/staging/sm750fb/sm750_accel.h | 4 +---
> >  4 files changed, 5 insertions(+), 9 deletions(-)
> >
>
> Does not apply to 7.3-rc1 :(
>
> Can you please rebase and resend?
>
> thanks,
> greg k-h