From: Bartlomiej Zolnierkiewicz b.zolnierkie@samsung.com Sent: Tuesday, June 9, 2020 6:44 AM To: Changming Liu liu.changm@northeastern.edu Cc: linux-fbdev@vger.kernel.org; dri-devel@lists.freedesktop.org; Lu, Long l.lu@northeastern.edu; yaohway@gmail.com Subject: Re: [Bug Report] drivers/video/fbdev/kyro/fbdev.c: unsigned integer wrap-around might cause unexpected behavior
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.
Thanks for the comments! These are valuable observations which show that hardware-driver interaction can play a role in unsigned wrap-around here. Indeed, there is no evidence to determine the wrap around is benign or not. Since these are just for legacy devices, I too would not take the risk to fix sth which is not broken.
Thanks again for your feedback, I learned a lot.
Best, Changming
Best regards,
Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Best, Changming Liu