On 7/23/20 4:21 PM, Greg Kroah-Hartman wrote: > On Wed, Jul 22, 2020 at 10:07:06AM +0200, Daniel Vetter wrote: >> On Tue, Jul 21, 2020 at 6:08 PM Greg Kroah-Hartman >> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: >>> >>> On Thu, Jul 16, 2020 at 08:27:21PM +0900, Tetsuo Handa wrote: >>>> On 2020/07/16 19:00, Daniel Vetter wrote: >>>>> On Thu, Jul 16, 2020 at 12:29:00AM +0900, Tetsuo Handa wrote: >>>>>> On 2020/07/16 0:12, Dan Carpenter wrote: >>>>>>> I've complained about integer overflows in fbdev for a long time... >>>>>>> >>>>>>> What I'd like to see is something like the following maybe. I don't >>>>>>> know how to get the vc_data in fbmem.c so it doesn't include your checks >>>>>>> for negative. >>>>>> >>>>>> Yes. Like I said "Thus, I consider that we need more sanity/constraints checks." at >>>>>> https://lore.kernel.org/lkml/b1e7dd6a-fc22-bba8-0abb-d3e779329bce@xxxxxxxxxxxxxxxxxxx/ , >>>>>> we want basic checks. That's a task for fbdev people who should be familiar with >>>>>> necessary constraints. >>>>> >>>>> I think the worldwide supply of people who understand fbdev and willing to >>>>> work on it is roughly 0. So if someone wants to fix this mess properly >>>>> (which likely means adding tons of over/underflow checks at entry points, >>>>> since you're never going to catch the driver bugs, there's too many and >>>>> not enough people who care) they need to fix this themselves. >>>> >>>> But I think we can enforce reasonable constraint which is much stricter than Dan's basic_checks() >>>> (which used INT_MAX). For example, do we need to accept var->{xres,yres} >= 1048576, for >>>> "32768 rows or cols" * "32 pixels per character" = 1045876 and vc_do_resize() accepts only >>>> rows and cols < 32768 ? >>>> >>>>> >>>>> Just to avoid confusion here. >>>>> >>>>>> Anyway, my two patches are small and low cost; can we apply these patches regardless >>>>>> of basic checks? >>>>> >>>>> Which two patches where? >>>> >>>> [PATCH v3] vt: Reject zero-sized screen buffer size. >>>> from https://lkml.kernel.org/r/20200712111013.11881-1-penguin-kernel@xxxxxxxxxxxxxxxxxxx >>> >>> This is now in my tree. >>> >>>> [PATCH v2] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins. >>>> from https://lkml.kernel.org/r/20200715015102.3814-1-penguin-kernel@xxxxxxxxxxxxxxxxxxx >>> >>> That should be taken by the fbdev maintainer, but I can take it too if >>> people want. >> >> Just missed this weeks pull request train and feeling like not worth >> making this an exception (it's been broken forever after all), so >> maybe best if you just add this to vt. >> >> Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx> >> >> Also this avoids the impression I know what's going on in fbdev code, >> maybe with sufficient abandon from my side someone will pop up who >> cares an fixes the bazillion of syzkaller issues we seem to have >> around console/vt and everything related. > > Great, will go queue it up now, thanks! Fine with me, thanks! PS I'll later queue the patch from George to drm-misc-next (after reading both fbdev patches in detail it seems that both are needed). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
