> From: Bartlomiej Zolnierkiewicz <b.zolnierkie@xxxxxxxxxxx> > Sent: Tuesday, June 9, 2020 6:44 AM > To: Changming Liu <liu.changm@xxxxxxxxxxxxxxxx> > Cc: linux-fbdev@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Lu, Long > <l.lu@xxxxxxxxxxxxxxxx>; yaohway@xxxxxxxxx > 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 > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
