| Message ID | 20230918205209.11709-3-s.shtylyov@omp.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix sloppy typing in the area copy | expand |
On 9/18/23 22:52, Sergey Shtylyov wrote: > In sys_copyarea(), when initializing *unsigned long const* bits_per_line > __u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which > might overflow __u32; multiplying by 8UL instead should fix that... > Also, that bits_per_line constant is used to advance *unsigned* src_idx > and dst_idx variables -- which might be overflowed as well; declaring > them as *unsigned long* should fix that too... > > Found by Linux Verification Center (linuxtesting.org) with the Svace static > analysis tool. > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > Cc: stable@vger.kernel.org > --- > drivers/video/fbdev/core/syscopyarea.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/core/syscopyarea.c b/drivers/video/fbdev/core/syscopyarea.c > index c1eda3190968..1035131383a6 100644 > --- a/drivers/video/fbdev/core/syscopyarea.c > +++ b/drivers/video/fbdev/core/syscopyarea.c > @@ -316,10 +316,11 @@ void sys_copyarea(struct fb_info *p, const struct fb_copyarea *area) > { > u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy; > u32 height = area->height, width = area->width; > - unsigned long const bits_per_line = p->fix.line_length*8u; > + unsigned long const bits_per_line = p->fix.line_length * 8UL; > unsigned long *base = NULL; > int bits = BITS_PER_LONG, bytes = bits >> 3; > - unsigned dst_idx = 0, src_idx = 0, rev_copy = 0; > + unsigned long dst_idx = 0, src_idx = 0; > + unsigned int rev_copy = 0; As mentioned in the other mail, both patches are not needed. Helge > > if (p->state != FBINFO_STATE_RUNNING) > return;
diff --git a/drivers/video/fbdev/core/syscopyarea.c b/drivers/video/fbdev/core/syscopyarea.c index c1eda3190968..1035131383a6 100644 --- a/drivers/video/fbdev/core/syscopyarea.c +++ b/drivers/video/fbdev/core/syscopyarea.c @@ -316,10 +316,11 @@ void sys_copyarea(struct fb_info *p, const struct fb_copyarea *area) { u32 dx = area->dx, dy = area->dy, sx = area->sx, sy = area->sy; u32 height = area->height, width = area->width; - unsigned long const bits_per_line = p->fix.line_length*8u; + unsigned long const bits_per_line = p->fix.line_length * 8UL; unsigned long *base = NULL; int bits = BITS_PER_LONG, bytes = bits >> 3; - unsigned dst_idx = 0, src_idx = 0, rev_copy = 0; + unsigned long dst_idx = 0, src_idx = 0; + unsigned int rev_copy = 0; if (p->state != FBINFO_STATE_RUNNING) return;
In sys_copyarea(), when initializing *unsigned long const* bits_per_line __u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which might overflow __u32; multiplying by 8UL instead should fix that... Also, that bits_per_line constant is used to advance *unsigned* src_idx and dst_idx variables -- which might be overflowed as well; declaring them as *unsigned long* should fix that too... Found by Linux Verification Center (linuxtesting.org) with the Svace static analysis tool. Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> Cc: stable@vger.kernel.org --- drivers/video/fbdev/core/syscopyarea.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)