[PATCH 1/2] video: fbdev: core: cfbcopyarea: fix sloppy typing

[Helge Deller]

On 9/19/23 20:55, Sergey Shtylyov wrote:
> On 9/19/23 10:05 AM, Helge Deller wrote:
>>> In cfb_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 at omp.ru>
>>> Cc: stable at vger.kernel.org
>>> ---
>>>    drivers/video/fbdev/core/cfbcopyarea.c | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/core/cfbcopyarea.c b/drivers/video/fbdev/core/cfbcopyarea.c
>>> index 6d4bfeecee35..b67ba69ea2fb 100644
>>> --- a/drivers/video/fbdev/core/cfbcopyarea.c
>>> +++ b/drivers/video/fbdev/core/cfbcopyarea.c
>>> @@ -382,10 +382,11 @@ void cfb_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;
>>
>> you wrote:
>>> __u32 typed fb_fix_screeninfo::line_length gets multiplied by 8u -- which
>>> might overflow __u32; multiplying by 8UL instead should fix that...
>>
>> This would only be true on 64-bit CPUs, where unsigned long is 64 bits,
>
>     Right, Svace was run with the arm64 and x86_64 configs -- and I forgot
> to make the emphasis on the 64-bit specifics here...
>
>> while on 32-bit CPUs, it's still 32 bits (same as _u32).
>
>     Yes, indeed. That *unsigned long const* doesn't seem justified
> at all then...
>
>> Instead we could make bits_per_line __u32 (or unsigned int) too.
>
>     Yes. Will you accept such a patch? :-)

Sure.

Helge