[Bug Report] drivers/video/fbdev/kyro/fbdev.c: unsigned integer wrap-around might cause unexpected behavior

[Bartlomiej Zolnierkiewicz]

Hi,

On 5/21/20 3:15 AM, Changming Liu wrote:
> Hi Bartlomiej,
> Greetings, I'm a first-year PhD student who is interested in the usage of UBSan for linux. 
> And after some experiments, I found that in drivers/video/fbdev/kyro/fbdev.c
> function kyro_dev_overlay_viewport_set, there is an unsigned integer overflow that might cause unexpected behavior.
> 
> More specifically, first at its caller, kyrofb_ioctl, after execution of copy_from_user at line 599, struct ol_viewport_set is filled with data from user space. 
> And the 4 32bit unsigned integers from it are passed into kyro_dev_overlay_viewport_set. In function kyro_dev_overlay_viewport_set, 
> x is added with ulWidth, y is added with ulHeight to transfer the length to the coordinate. 
> And the result coordinate might overflow and wrap around. And it is passed into function SetOverlayViewPort.
> 
> It appears that in function SetOverlayViewPort, these values are treated as the coordinate of the bottom-right point and the wrap-around is not checked.(I might miss something).
> 
> Due to the lack of knowledge of the interaction between this module and the user space, I'm not able to assess if this is a benign wrap-around or whether the wrap-around could happen at all. 
> I'd appreciate for you comment on this issue, this could help me understand linux and unsigned wrap around a lot.
> 
> Looking forward to your valuable response!

It seems that wrap-around can indeed happen but I'm not sure
what are the exact consequences of it (SetOverlayViewPort() is
quite complicated and I also don't know how hardware would
react to improper settings).

kyrofb driver is for legacy devices and is not actively
maintained so I worry that without somebody with the access
to hardware and time to investigate it further I cannot do
much about the problem.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> Best,
> Changming Liu
>